Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow @RequestPart user-defined POJOs #314

Merged
merged 28 commits into from
Apr 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
beba48b
Add Pojo and failing test to FeignClientTests
darrenfoong Mar 27, 2020
b629e13
Add SpringPojoFormEncoder
darrenfoong Mar 28, 2020
59df0b6
Fix configuration
darrenfoong Mar 28, 2020
f965475
Update Pojo
darrenfoong Mar 28, 2020
93b7f80
Implement multipartPojo
darrenfoong Mar 28, 2020
a8b7cb0
Does not work if Pojo has one field: this is crazy
darrenfoong Mar 28, 2020
ce3b96f
Disable expansion for multipart/form-data
darrenfoong Mar 28, 2020
b66c3ba
Remove Pojo
darrenfoong Mar 28, 2020
9cdfc49
Tidy code
darrenfoong Mar 29, 2020
1a69560
Add testSinglePojoRequestPart
darrenfoong Mar 29, 2020
1a917e2
Update testMultiplePojoRequestPart
darrenfoong Mar 29, 2020
0e3b5fa
Update testMultiplePojoRequestPart
darrenfoong Mar 29, 2020
5312520
Add testRequestPartWithListOfPojosAndListOfMultipartFiles
darrenfoong Mar 29, 2020
a7d98fe
Fix Checkstyle errors
darrenfoong Mar 29, 2020
c8c1f0b
Update license year and authors
darrenfoong Mar 29, 2020
e33e0fd
Tidy isApplicable
darrenfoong Mar 29, 2020
d18b7ab
Refactor isApplicable
darrenfoong Mar 29, 2020
974bf01
Rename classes and methods
darrenfoong Mar 29, 2020
75066f9
Change variable names
darrenfoong Mar 29, 2020
9a9112f
Rename PojoFormWriter to AbstractFormWriter
darrenfoong Apr 9, 2020
1b515cb
Rename method
darrenfoong Apr 9, 2020
a5e07ba
Use ObjectProvider
darrenfoong Apr 10, 2020
33a279b
Change constructor of SpringEncoder to move SpringPojoFormEncoder to …
darrenfoong Apr 10, 2020
93050f3
Rename variables
darrenfoong Apr 10, 2020
dcc120e
Remove unused variable
darrenfoong Apr 11, 2020
a1440a4
Extract springEncoder() method
darrenfoong Apr 11, 2020
3b7f9b2
Restore previous constructor
darrenfoong Apr 11, 2020
1799ade
Extract isMultipartFormData() method
darrenfoong Apr 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -27,17 +27,21 @@
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;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
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;
Expand All @@ -51,9 +55,12 @@
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 {
Expand Down Expand Up @@ -83,16 +90,18 @@ public Decoder feignDecoder() {
@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());
Expand Down Expand Up @@ -144,6 +153,18 @@ public Module pageJacksonModule() {
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 {
Expand All @@ -158,4 +179,16 @@ public Feign.Builder feignHystrixBuilder() {

}

private class SpringPojoFormEncoder extends SpringFormEncoder {

SpringPojoFormEncoder(AbstractFormWriter formWriter) {
super();

MultipartFormContentProcessor processor = (MultipartFormContentProcessor) getContentProcessor(
MULTIPART);
processor.addFirstWriter(formWriter);
}

}

}
Original file line number Diff line number Diff line change
@@ -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);
}
}

}
darrenfoong marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -54,16 +54,24 @@
* @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,
darrenfoong marked this conversation as resolved.
Show resolved Hide resolved
ObjectFactory<HttpMessageConverters> messageConverters) {
this.springFormEncoder = springFormEncoder;
this.messageConverters = messageConverters;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -28,6 +28,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import feign.Contract;
import feign.Feign;
Expand All @@ -42,6 +43,7 @@
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;
Expand All @@ -54,6 +56,7 @@
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;
Expand All @@ -72,6 +75,7 @@
* @author Olga Maciaszek-Sharma
* @author Aaron Whiteside
* @author Artyom Romanenko
* @author Darren Foong
*/
public class SpringMvcContract extends Contract.BaseContract
implements ResourceLoaderAware {
Expand Down Expand Up @@ -291,7 +295,8 @@ protected boolean processAnnotationsOnParameter(MethodMetadata data,
}
}

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)) {
Expand Down Expand Up @@ -388,6 +393,18 @@ private boolean shouldAddParameterName(int parameterIndex, Type[] parameterTypes
&& 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.
*/
Expand Down