Allow @RequestPart user-defined POJOs (#314)

* Add Pojo and failing test to FeignClientTests

* Add SpringPojoFormEncoder

* Fix configuration

* Update Pojo

* Implement multipartPojo

* Does not work if Pojo has one field: this is crazy

* Disable expansion for multipart/form-data

* Remove Pojo

* Tidy code

* Add testSinglePojoRequestPart

* Update testMultiplePojoRequestPart

* Update testMultiplePojoRequestPart

* Add testRequestPartWithListOfPojosAndListOfMultipartFiles

* Fix Checkstyle errors

* Update license year and authors

* Tidy isApplicable

* Refactor isApplicable

* Rename classes and methods

* Change variable names

* Rename PojoFormWriter to AbstractFormWriter

* Rename method

* Use ObjectProvider

* Change constructor of SpringEncoder to move SpringPojoFormEncoder to FeignClientsConfiguration

* Rename variables

* Remove unused variable

* Extract springEncoder() method

* Restore previous constructor

* Extract isMultipartFormData() method

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 the original author or authors.
+ * Copyright 2013-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,10 +27,13 @@ import feign.Logger;
 import feign.Retryer;
 import feign.codec.Decoder;
 import feign.codec.Encoder;
+import feign.form.MultipartFormContentProcessor;
+import feign.form.spring.SpringFormEncoder;
 import feign.hystrix.HystrixFeign;
 import feign.optionals.OptionalDecoder;
 
 import org.springframework.beans.factory.ObjectFactory;
+import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -38,6 +41,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClas
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.data.web.SpringDataWebProperties;
 import org.springframework.boot.autoconfigure.http.HttpMessageConverters;
+import org.springframework.cloud.openfeign.support.AbstractFormWriter;
 import org.springframework.cloud.openfeign.support.PageJacksonModule;
 import org.springframework.cloud.openfeign.support.PageableSpringEncoder;
 import org.springframework.cloud.openfeign.support.ResponseEntityDecoder;
@@ -51,9 +55,12 @@ import org.springframework.core.convert.ConversionService;
 import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.format.support.FormattingConversionService;
 
+import static feign.form.ContentType.MULTIPART;
+
 /**
  * @author Dave Syer
  * @author Venil Noronha
+ * @author Darren Foong
  */
 @Configuration(proxyBeanMethods = false)
 public class FeignClientsConfiguration {
@@ -83,16 +90,18 @@ public class FeignClientsConfiguration {
 	@Bean
 	@ConditionalOnMissingBean
 	@ConditionalOnMissingClass("org.springframework.data.domain.Pageable")
-	public Encoder feignEncoder() {
-		return new SpringEncoder(this.messageConverters);
+	public Encoder feignEncoder(ObjectProvider<AbstractFormWriter> formWriterProvider) {
+		return springEncoder(formWriterProvider);
 	}
 
 	@Bean
 	@ConditionalOnClass(name = "org.springframework.data.domain.Pageable")
 	@ConditionalOnMissingBean
-	public Encoder feignEncoderPageable() {
+	public Encoder feignEncoderPageable(
+			ObjectProvider<AbstractFormWriter> formWriterProvider) {
 		PageableSpringEncoder encoder = new PageableSpringEncoder(
-				new SpringEncoder(this.messageConverters));
+				springEncoder(formWriterProvider));
+
 		if (springDataWebProperties != null) {
 			encoder.setPageParameter(
 					springDataWebProperties.getPageable().getPageParameter());
@@ -144,6 +153,18 @@ public class FeignClientsConfiguration {
 		return new PageJacksonModule();
 	}
 
+	private Encoder springEncoder(ObjectProvider<AbstractFormWriter> formWriterProvider) {
+		AbstractFormWriter formWriter = formWriterProvider.getIfAvailable();
+
+		if (formWriter != null) {
+			return new SpringEncoder(new SpringPojoFormEncoder(formWriter),
+					this.messageConverters);
+		}
+		else {
+			return new SpringEncoder(new SpringFormEncoder(), this.messageConverters);
+		}
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@ConditionalOnClass({ HystrixCommand.class, HystrixFeign.class })
 	protected static class HystrixFeignConfiguration {
@@ -158,4 +179,16 @@ public class FeignClientsConfiguration {
 
 	}
 
+	private class SpringPojoFormEncoder extends SpringFormEncoder {
+
+		SpringPojoFormEncoder(AbstractFormWriter formWriter) {
+			super();
+
+			MultipartFormContentProcessor processor = (MultipartFormContentProcessor) getContentProcessor(
+					MULTIPART);
+			processor.addFirstWriter(formWriter);
+		}
+
+	}
+
 }

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

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2013-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.openfeign.support;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.function.Predicate;
+
+import feign.codec.EncodeException;
+import feign.form.multipart.AbstractWriter;
+import feign.form.multipart.Output;
+import feign.form.util.PojoUtil;
+
+import org.springframework.http.MediaType;
+import org.springframework.web.multipart.MultipartFile;
+
+import static feign.form.ContentProcessor.CRLF;
+import static feign.form.util.PojoUtil.isUserPojo;
+
+/**
+ * @author Darren Foong
+ */
+public abstract class AbstractFormWriter extends AbstractWriter {
+
+	@Override
+	public boolean isApplicable(Object object) {
+		return !isTypeOrCollection(object, o -> o instanceof MultipartFile)
+				&& isTypeOrCollection(object, PojoUtil::isUserPojo);
+	}
+
+	@Override
+	public void write(Output output, String key, Object object) throws EncodeException {
+		try {
+			String string = new StringBuilder()
+					.append("Content-Disposition: form-data; name=\"").append(key)
+					.append('"').append(CRLF).append("Content-Type: ")
+					.append(getContentType()).append("; charset=")
+					.append(output.getCharset().name()).append(CRLF).append(CRLF)
+					.append(writeAsString(object)).toString();
+
+			output.write(string);
+		}
+		catch (IOException e) {
+			throw new EncodeException(e.getMessage());
+		}
+	}
+
+	protected abstract MediaType getContentType();
+
+	protected abstract String writeAsString(Object object) throws IOException;
+
+	private boolean isTypeOrCollection(Object object, Predicate<Object> isType) {
+		if (object.getClass().isArray()) {
+			Object[] array = (Object[]) object;
+
+			return array.length > 1 && isType.test(array[0]);
+		}
+		else if (object instanceof Iterable) {
+			Iterable<?> iterable = (Iterable<?>) object;
+			Iterator<?> iterator = iterable.iterator();
+
+			return iterator.hasNext() && isType.test(iterator.next());
+		}
+		else {
+			return isType.test(object);
+		}
+	}
+
+}

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

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2013-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.openfeign.support;
+
+import java.io.IOException;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.MediaType;
+import org.springframework.stereotype.Component;
+
+/**
+ * @author Darren Foong
+ */
+@Component
+public class JsonFormWriter extends AbstractFormWriter {
+
+	@Autowired
+	private ObjectMapper objectMapper;
+
+	@Override
+	protected MediaType getContentType() {
+		return MediaType.APPLICATION_JSON;
+	}
+
+	@Override
+	protected String writeAsString(Object object) throws IOException {
+		return objectMapper.writeValueAsString(object);
+	}
+
+}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 the original author or authors.
+ * Copyright 2013-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -54,16 +54,24 @@ import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHead
  * @author Scien Jus
  * @author Ahmad Mozafarnia
  * @author Aaron Whiteside
+ * @author Darren Foong
  */
 public class SpringEncoder implements Encoder {
 
 	private static final Log log = LogFactory.getLog(SpringEncoder.class);
 
-	private final SpringFormEncoder springFormEncoder = new SpringFormEncoder();
+	private final SpringFormEncoder springFormEncoder;
 
 	private final ObjectFactory<HttpMessageConverters> messageConverters;
 
 	public SpringEncoder(ObjectFactory<HttpMessageConverters> messageConverters) {
+		this.springFormEncoder = new SpringFormEncoder();
+		this.messageConverters = messageConverters;
+	}
+
+	public SpringEncoder(SpringFormEncoder springFormEncoder,
+			ObjectFactory<HttpMessageConverters> messageConverters) {
+		this.springFormEncoder = springFormEncoder;
 		this.messageConverters = messageConverters;
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 the original author or authors.
+ * Copyright 2013-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@ import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import feign.Contract;
 import feign.Feign;
@@ -42,6 +43,7 @@ import org.springframework.cloud.openfeign.annotation.QueryMapParameterProcessor
 import org.springframework.cloud.openfeign.annotation.RequestHeaderParameterProcessor;
 import org.springframework.cloud.openfeign.annotation.RequestParamParameterProcessor;
 import org.springframework.cloud.openfeign.annotation.RequestPartParameterProcessor;
+import org.springframework.cloud.openfeign.encoding.HttpEncoding;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.ResourceLoaderAware;
 import org.springframework.core.DefaultParameterNameDiscoverer;
@@ -54,6 +56,7 @@ import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.ResourceLoader;
+import org.springframework.http.MediaType;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.RequestMapping;
@@ -72,6 +75,7 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.findMerg
  * @author Olga Maciaszek-Sharma
  * @author Aaron Whiteside
  * @author Artyom Romanenko
+ * @author Darren Foong
  */
 public class SpringMvcContract extends Contract.BaseContract
 		implements ResourceLoaderAware {
@@ -291,7 +295,8 @@ public class SpringMvcContract extends Contract.BaseContract
 			}
 		}
 
-		if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null) {
+		if (!isMultipartFormData(data) && isHttpAnnotation
+				&& data.indexToExpander().get(paramIndex) == null) {
 			TypeDescriptor typeDescriptor = createTypeDescriptor(method, paramIndex);
 			if (this.conversionService.canConvert(typeDescriptor,
 					STRING_TYPE_DESCRIPTOR)) {
@@ -388,6 +393,18 @@ public class SpringMvcContract extends Contract.BaseContract
 				&& parameterTypes != null && parameterTypes.length > parameterIndex;
 	}
 
+	private boolean isMultipartFormData(MethodMetadata data) {
+		Collection<String> contentTypes = data.template().headers()
+				.get(HttpEncoding.CONTENT_TYPE);
+
+		if (contentTypes != null && !contentTypes.isEmpty()) {
+			String type = contentTypes.iterator().next();
+			return Objects.equals(MediaType.valueOf(type), MediaType.MULTIPART_FORM_DATA);
+		}
+
+		return false;
+	}
+
 	/**
 	 * @deprecated Not used internally anymore. Will be removed in the future.
 	 */

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2019 the original author or authors.
+ * Copyright 2013-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -74,7 +74,9 @@ import org.springframework.cloud.openfeign.EnableFeignClients;
 import org.springframework.cloud.openfeign.FeignClient;
 import org.springframework.cloud.openfeign.FeignFormatterRegistrar;
 import org.springframework.cloud.openfeign.ribbon.LoadBalancerFeignClient;
+import org.springframework.cloud.openfeign.support.AbstractFormWriter;
 import org.springframework.cloud.openfeign.support.FallbackCommand;
+import org.springframework.cloud.openfeign.support.JsonFormWriter;
 import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -107,6 +109,7 @@ import static org.hamcrest.core.IsInstanceOf.instanceOf;
  * @author Erik Kringen
  * @author Halvdan Hoem Grelland
  * @author Aaron Whiteside
+ * @author Darren Foong
  */
 @RunWith(SpringJUnit4ClassRunner.class)
 @SpringBootTest(classes = FeignClientTests.Application.class,
@@ -450,6 +453,12 @@ public class FeignClientTests {
 		assertThat(response).isEqualTo("abc");
 	}
 
+	@Test
+	public void testSinglePojoRequestPart() {
+		String response = this.multipartClient.singlePojoPart(new Hello(HELLO_WORLD_1));
+		assertThat(response).isEqualTo(HELLO_WORLD_1);
+	}
+
 	@Test
 	public void testMultipleRequestParts() {
 		MockMultipartFile file = new MockMultipartFile("file", "hello.bin", null,
@@ -458,6 +467,17 @@ public class FeignClientTests {
 		assertThat(response).isEqualTo("abc123hello.bin");
 	}
 
+	@Test
+	public void testMultiplePojoRequestParts() {
+		Hello pojo1 = new Hello(HELLO_WORLD_1);
+		Hello pojo2 = new Hello(OI_TERRA_2);
+		MockMultipartFile file = new MockMultipartFile("file", "hello.bin", null,
+				"hello".getBytes());
+		String response = this.multipartClient.multipartPojo("abc", "123", pojo1, pojo2,
+				file);
+		assertThat(response).isEqualTo("abc123hello world 1oi terra 2hello.bin");
+	}
+
 	@Test
 	public void testRequestPartWithListOfMultipartFiles() {
 		List<MultipartFile> multipartFiles = Arrays.asList(
@@ -471,6 +491,20 @@ public class FeignClientTests {
 		assertThat(fileNames).contains("hello1.bin", "hello2.bin");
 	}
 
+	@Test
+	public void testRequestPartWithListOfPojosAndListOfMultipartFiles() {
+		Hello pojo1 = new Hello(HELLO_WORLD_1);
+		Hello pojo2 = new Hello(OI_TERRA_2);
+		MockMultipartFile file1 = new MockMultipartFile("file1", "hello1.bin", null,
+				"hello".getBytes());
+		MockMultipartFile file2 = new MockMultipartFile("file2", "hello2.bin", null,
+				"hello".getBytes());
+		String response = this.multipartClient
+				.requestPartListOfPojosAndListOfMultipartFiles(
+						Arrays.asList(pojo1, pojo2), Arrays.asList(file1, file2));
+		assertThat(response).isEqualTo("hello world 1oi terra 2hello1.binhello2.bin");
+	}
+
 	@Test
 	public void testRequestBodyWithSingleMultipartFile() {
 		String partName = UUID.randomUUID().toString();
@@ -671,6 +705,11 @@ public class FeignClientTests {
 				produces = MediaType.TEXT_PLAIN_VALUE)
 		String singlePart(@RequestPart("hello") String hello);
 
+		@RequestMapping(method = RequestMethod.POST, path = "/singlePojoPart",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String singlePojoPart(@RequestPart("hello") Hello hello);
+
 		@RequestMapping(method = RequestMethod.POST, path = "/multipart",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
@@ -678,6 +717,14 @@ public class FeignClientTests {
 				@RequestPart("world") String world,
 				@RequestPart("file") MultipartFile file);
 
+		@RequestMapping(method = RequestMethod.POST, path = "/multipartPojo",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String multipartPojo(@RequestPart("hello") String hello,
+				@RequestPart("world") String world, @RequestPart("pojo1") Hello pojo1,
+				@RequestPart("pojo2") Hello pojo2,
+				@RequestPart("file") MultipartFile file);
+
 		@RequestMapping(method = RequestMethod.POST, path = "/multipartNames",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
@@ -690,6 +737,13 @@ public class FeignClientTests {
 		String requestPartListOfMultipartFilesReturnsFileNames(
 				@RequestPart("files") List<MultipartFile> files);
 
+		@RequestMapping(method = RequestMethod.POST, path = "/multipartPojosFiles",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String requestPartListOfPojosAndListOfMultipartFiles(
+				@RequestPart("pojos") List<Hello> pojos,
+				@RequestPart("files") List<MultipartFile> files);
+
 		@RequestMapping(method = RequestMethod.POST, path = "/multipartNames",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
@@ -924,6 +978,11 @@ public class FeignClientTests {
 			};
 		}
 
+		@Bean
+		public AbstractFormWriter jsonFormWriter() {
+			return new JsonFormWriter();
+		}
+
 		@RequestMapping(method = RequestMethod.GET, path = "/hello")
 		public Hello getHello() {
 			return new Hello(HELLO_WORLD_1);
@@ -1029,10 +1088,17 @@ public class FeignClientTests {
 		@RequestMapping(method = RequestMethod.POST, path = "/singlePart",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
-		String multipart(@RequestPart("hello") String hello) {
+		String singlePart(@RequestPart("hello") String hello) {
 			return hello;
 		}
 
+		@RequestMapping(method = RequestMethod.POST, path = "/singlePojoPart",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String singlePojoPart(@RequestPart("hello") Hello hello) {
+			return hello.getMessage();
+		}
+
 		@RequestMapping(method = RequestMethod.POST, path = "/multipart",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
@@ -1042,6 +1108,17 @@ public class FeignClientTests {
 			return hello + world + file.getOriginalFilename();
 		}
 
+		@RequestMapping(method = RequestMethod.POST, path = "/multipartPojo",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String multipartPojo(@RequestPart("hello") String hello,
+				@RequestPart("world") String world, @RequestPart("pojo1") Hello pojo1,
+				@RequestPart("pojo2") Hello pojo2,
+				@RequestPart("file") MultipartFile file) {
+			return hello + world + pojo1.getMessage() + pojo2.getMessage()
+					+ file.getOriginalFilename();
+		}
+
 		@RequestMapping(method = RequestMethod.POST, path = "/multipartNames",
 				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
 				produces = MediaType.TEXT_PLAIN_VALUE)
@@ -1058,6 +1135,25 @@ public class FeignClientTests {
 					.collect(Collectors.joining(","));
 		}
 
+		@RequestMapping(method = RequestMethod.POST, path = "/multipartPojosFiles",
+				consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
+				produces = MediaType.TEXT_PLAIN_VALUE)
+		String requestPartListOfPojosAndListOfMultipartFiles(
+				@RequestPart("pojos") List<Hello> pojos,
+				@RequestPart("files") List<MultipartFile> files) {
+			StringBuilder result = new StringBuilder();
+
+			for (Hello pojo : pojos) {
+				result.append(pojo.getMessage());
+			}
+
+			for (MultipartFile file : files) {
+				result.append(file.getOriginalFilename());
+			}
+
+			return result.toString();
+		}
+
 	}
 
 	public static class Hello {