Skip to content

Support Spring formatting annotations for params #48

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Aug 15, 2018
Merged

Support Spring formatting annotations for params #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
84e703d
Add factory for Param.Expander using ConversionService. Instances use…
grelland Jul 22, 2018
8d1de6b
Minor: comment
grelland Jul 22, 2018
ae9dda5
Fix expansion of Iterable params - provide converting Param.Expander …
grelland Jul 25, 2018
d52bd5a
Add test for param with @DateTimeFormat annotation.
grelland Jul 25, 2018
6490be9
Minor: formatting + add author tag
grelland Jul 26, 2018
5e42059
Simplified comment
grelland Jul 26, 2018
81640e7
Minor: revert auto-format, change test setup a tiny bit
grelland Jul 26, 2018
ffb223a
Replace TODO with Deprecation
grelland Jul 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 62 additions & 6 deletions ...ign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -35,9 +36,11 @@
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.ResourceLoaderAware;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.convert.ConversionService;
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;
Expand All @@ -58,6 +61,7 @@
/**
* @author Spencer Gibb
* @author Abhijit Sarkar
* @author Halvdan Hoem Grelland
*/
public class SpringMvcContract extends Contract.BaseContract
implements ResourceLoaderAware {
Expand All @@ -66,13 +70,18 @@ public class SpringMvcContract extends Contract.BaseContract

private static final String CONTENT_TYPE = "Content-Type";

private static final TypeDescriptor STRING_TYPE_DESCRIPTOR =
TypeDescriptor.valueOf(String.class);
private static final TypeDescriptor ITERABLE_TYPE_DESCRIPTOR =
TypeDescriptor.valueOf(Iterable.class);

private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer();

private final Map<Class<? extends Annotation>, AnnotatedParameterProcessor> annotatedArgumentProcessors;
private final Map<String, Method> processedMethods = new HashMap<>();

private final ConversionService conversionService;
private final Param.Expander expander;
private final ConvertingExpanderFactory convertingExpanderFactory;
private ResourceLoader resourceLoader = new DefaultResourceLoader();

public SpringMvcContract() {
Expand Down Expand Up @@ -100,7 +109,7 @@ public SpringMvcContract(
}
this.annotatedArgumentProcessors = toAnnotatedArgumentProcessorMap(processors);
this.conversionService = conversionService;
this.expander = new ConvertingExpander(conversionService);
this.convertingExpanderFactory = new ConvertingExpanderFactory(conversionService);
}

@Override
Expand Down Expand Up @@ -239,14 +248,40 @@ protected boolean processAnnotationsOnParameter(MethodMetadata data,
processParameterAnnotation, method);
}
}
if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null
&& this.conversionService.canConvert(
method.getParameterTypes()[paramIndex], String.class)) {
data.indexToExpander().put(paramIndex, this.expander);

if (isHttpAnnotation && data.indexToExpander().get(paramIndex) == null) {
TypeDescriptor typeDescriptor = createTypeDescriptor(method, paramIndex);
if (conversionService.canConvert(typeDescriptor, STRING_TYPE_DESCRIPTOR)) {
Param.Expander expander =
convertingExpanderFactory.getExpander(typeDescriptor);
if (expander != null) {
data.indexToExpander().put(paramIndex, expander);
}
}
}
return isHttpAnnotation;
}

private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) {
Parameter parameter = method.getParameters()[paramIndex];
MethodParameter methodParameter = MethodParameter.forParameter(parameter);
TypeDescriptor typeDescriptor = new TypeDescriptor(methodParameter);

// Feign applies the Param.Expander to each element of an Iterable, so in those
// cases we need to provide a TypeDescriptor of the element.
if (typeDescriptor.isAssignableTo(ITERABLE_TYPE_DESCRIPTOR)) {
TypeDescriptor elementTypeDescriptor =
typeDescriptor.getElementTypeDescriptor();

checkState(elementTypeDescriptor != null,
"Could not resolve element type of Iterable type %s. Not declared?",
typeDescriptor);

typeDescriptor = elementTypeDescriptor;
}
return typeDescriptor;
}

private void parseProduces(MethodMetadata md, Method method,
RequestMapping annotation) {
String[] serverProduces = annotation.produces();
Expand Down Expand Up @@ -361,6 +396,10 @@ public Collection<String> setTemplateParameter(String name,
}
}

/**
* @deprecated Not used internally anymore. Will be removed in the future.
*/
@Deprecated
public static class ConvertingExpander implements Param.Expander {

private final ConversionService conversionService;
Expand All @@ -375,4 +414,21 @@ public String expand(Object value) {
}

}

private static class ConvertingExpanderFactory {

private final ConversionService conversionService;

ConvertingExpanderFactory(ConversionService conversionService) {
this.conversionService = conversionService;
}

Param.Expander getExpander(TypeDescriptor typeDescriptor) {
return value -> {
Object converted = this.conversionService.convert(
value, typeDescriptor, STRING_TYPE_DESCRIPTOR);
return (String) converted;
};
}
}
}
79 changes: 78 additions & 1 deletion ...ore/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,24 @@

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;

import feign.Param;
import org.junit.Before;
import org.junit.Test;

import org.springframework.core.convert.ConversionService;
import org.springframework.format.annotation.DateTimeFormat;
import org.springframework.format.annotation.NumberFormat;
import org.springframework.format.number.NumberStyleFormatter;
import org.springframework.format.support.FormattingConversionServiceFactoryBean;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.util.MultiValueMap;
Expand All @@ -48,6 +60,7 @@

/**
* @author chadjaros
* @author Halvdan Hoem Grelland
*/
public class SpringMvcContractTests {
private static final Class<?> EXECUTABLE_TYPE;
Expand All @@ -67,7 +80,12 @@ public class SpringMvcContractTests {

@Before
public void setup() {
this.contract = new SpringMvcContract();
FormattingConversionServiceFactoryBean conversionServiceFactoryBean
= new FormattingConversionServiceFactoryBean();
conversionServiceFactoryBean.afterPropertiesSet();
ConversionService conversionService = conversionServiceFactoryBean.getObject();

this.contract = new SpringMvcContract(Collections.emptyList(), conversionService);
}

@Test
Expand Down Expand Up @@ -255,6 +273,47 @@ public void testProcessAnnotations_Aliased() throws Exception {
data.template().queries().get("amount").iterator().next());
}

@Test
public void testProcessAnnotations_DateTimeFormatParam() throws Exception {
Method method = TestTemplate_DateTimeFormatParameter.class.getDeclaredMethod(
"getTest", LocalDateTime.class);
MethodMetadata data = this.contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);

Param.Expander expander = data.indexToExpander().get(0);
assertNotNull(expander);

LocalDateTime input = LocalDateTime.of(2001, 10, 12, 23, 56, 3);

DateTimeFormatter formatter = DateTimeFormatter.ofPattern(
TestTemplate_DateTimeFormatParameter.CUSTOM_PATTERN);

String expected = formatter.format(input);

assertEquals(expected, expander.expand(input));
}

@Test
public void testProcessAnnotations_NumberFormatParam() throws Exception {
Method method = TestTemplate_NumberFormatParameter.class.getDeclaredMethod(
"getTest", BigDecimal.class);
MethodMetadata data = this.contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);

Param.Expander expander = data.indexToExpander().get(0);
assertNotNull(expander);

NumberStyleFormatter formatter = new NumberStyleFormatter(
TestTemplate_NumberFormatParameter.CUSTOM_PATTERN);

BigDecimal input = BigDecimal.valueOf(1220.345);

String expected = formatter.print(input, Locale.getDefault());
String actual = expander.expand(input);

assertEquals(expected, actual);
}

@Test
public void testProcessAnnotations_Advanced2() throws Exception {
Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest");
Expand Down Expand Up @@ -539,6 +598,24 @@ ResponseEntity<TestObject> getTestFallback(@RequestHeader String Authorization,
TestObject getTest();
}

public interface TestTemplate_DateTimeFormatParameter {

String CUSTOM_PATTERN = "dd-MM-yyyy HH:mm";

@RequestMapping(method = RequestMethod.GET)
String getTest(@RequestParam(name = "localDateTime")
@DateTimeFormat(pattern = CUSTOM_PATTERN) LocalDateTime localDateTime);
}

public interface TestTemplate_NumberFormatParameter {

String CUSTOM_PATTERN = "$###,###.###";

@RequestMapping(method = RequestMethod.GET)
String getTest(@RequestParam("amount")
@NumberFormat(pattern = CUSTOM_PATTERN) BigDecimal amount);
}

@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE)
public class TestObject {

Expand Down
24 changes: 24 additions & 0 deletions ...nfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.text.ParseException;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -51,6 +52,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.format.Formatter;
import org.springframework.format.FormatterRegistry;
import org.springframework.format.annotation.DateTimeFormat;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -93,6 +95,7 @@
* @author Spencer Gibb
* @author Jakub Narloch
* @author Erik Kringen
* @author Halvdan Hoem Grelland
*/
@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = FeignClientTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = {
Expand Down Expand Up @@ -187,6 +190,11 @@ protected interface TestClient {
@RequestMapping(method = RequestMethod.GET, path = "/helloparams")
List<String> getParams(@RequestParam("params") List<String> params);

@RequestMapping(method = RequestMethod.GET, path = "/formattedparams")
List<LocalDate> getFormattedParams(
@RequestParam("params")
@DateTimeFormat(pattern = "dd-MM-yyyy") List<LocalDate> params);

@RequestMapping(method = RequestMethod.GET, path = "/hellos")
HystrixCommand<List<Hello>> getHellosHystrix();

Expand Down Expand Up @@ -441,6 +449,13 @@ public List<String> getParams(@RequestParam("params") List<String> params) {
return params;
}

@RequestMapping(method = RequestMethod.GET, path = "/formattedparams")
public List<LocalDate> getFormattedParams(
@RequestParam("params")
@DateTimeFormat(pattern = "dd-MM-yyyy") List<LocalDate> params) {
return params;
}

@RequestMapping(method = RequestMethod.GET, path = "/noContent")
ResponseEntity<Void> noContent() {
return ResponseEntity.noContent().build();
Expand Down Expand Up @@ -583,6 +598,15 @@ public void testParams() {
assertEquals("params size was wrong", list.size(), params.size());
}

@Test
public void testFormattedParams() {
List<LocalDate> list = Arrays.asList(
LocalDate.of(2001, 1, 1), LocalDate.of(2018, 6, 10));
List<LocalDate> params = this.testClient.getFormattedParams(list);
assertNotNull("params was null", params);
assertEquals("params not converted correctly", list, params);
}

@Test
public void testHystrixCommand() throws NoSuchMethodException {
HystrixCommand<List<Hello>> command = this.testClient.getHellosHystrix();
Expand Down