Revert "Add Feign expanders based on Spring conversion service"
This reverts commit 3f34ccc8fb. We
need to discuss a bit more (that thread local is probably unavoidable
given the design of Feign, but I don't like it yet).
See gh-841
This commit is contained in:
@@ -33,15 +33,12 @@ import org.springframework.cloud.netflix.feign.annotation.RequestParamParameterP
|
||||
import org.springframework.core.DefaultParameterNameDiscoverer;
|
||||
import org.springframework.core.ParameterNameDiscoverer;
|
||||
import org.springframework.core.annotation.AnnotationUtils;
|
||||
import org.springframework.core.convert.ConversionService;
|
||||
import org.springframework.core.convert.support.DefaultConversionService;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
|
||||
import feign.Contract;
|
||||
import feign.Feign;
|
||||
import feign.MethodMetadata;
|
||||
import feign.Param;
|
||||
|
||||
import static feign.Util.checkState;
|
||||
import static feign.Util.emptyToNull;
|
||||
@@ -61,24 +58,14 @@ public class SpringMvcContract extends Contract.BaseContract {
|
||||
private final Map<Class<? extends Annotation>, AnnotatedParameterProcessor> annotatedArgumentProcessors;
|
||||
private final Map<String, Method> processedMethods = new HashMap<>();
|
||||
|
||||
private final ConversionService conversionService;
|
||||
|
||||
public SpringMvcContract() {
|
||||
this(Collections.<AnnotatedParameterProcessor> emptyList());
|
||||
}
|
||||
|
||||
public SpringMvcContract(
|
||||
List<AnnotatedParameterProcessor> annotatedParameterProcessors) {
|
||||
this(annotatedParameterProcessors, new DefaultConversionService());
|
||||
}
|
||||
|
||||
public SpringMvcContract(
|
||||
List<AnnotatedParameterProcessor> annotatedParameterProcessors,
|
||||
ConversionService conversionService) {
|
||||
Assert.notNull(annotatedParameterProcessors,
|
||||
"Parameter processors can not be null.");
|
||||
Assert.notNull(conversionService,
|
||||
"ConversionService can not be null.");
|
||||
|
||||
List<AnnotatedParameterProcessor> processors;
|
||||
if (!annotatedParameterProcessors.isEmpty()) {
|
||||
@@ -88,8 +75,6 @@ public class SpringMvcContract extends Contract.BaseContract {
|
||||
processors = getDefaultAnnotatedArgumentsProcessors();
|
||||
}
|
||||
this.annotatedArgumentProcessors = toAnnotatedArgumentProcessorMap(processors);
|
||||
this.conversionService = conversionService;
|
||||
ConvertingExpander.CONVERSION_SERVICE.set(conversionService);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -198,11 +183,6 @@ public class SpringMvcContract extends Contract.BaseContract {
|
||||
processParameterAnnotation);
|
||||
}
|
||||
}
|
||||
if (isHttpAnnotation && data.indexToExpanderClass().get(paramIndex) == null
|
||||
&& this.conversionService.canConvert(
|
||||
method.getParameterTypes()[paramIndex], String.class)) {
|
||||
data.indexToExpanderClass().put(paramIndex, ConvertingExpander.class);
|
||||
}
|
||||
return isHttpAnnotation;
|
||||
}
|
||||
|
||||
@@ -312,23 +292,4 @@ public class SpringMvcContract extends Contract.BaseContract {
|
||||
}
|
||||
}
|
||||
|
||||
public static class ConvertingExpander implements Param.Expander {
|
||||
static final ThreadLocal<ConversionService> CONVERSION_SERVICE = new ThreadLocal<ConversionService>() {
|
||||
protected ConversionService initialValue() {
|
||||
return new DefaultConversionService();
|
||||
}
|
||||
};
|
||||
|
||||
private final ConversionService conversionService;
|
||||
|
||||
public ConvertingExpander() {
|
||||
this.conversionService = CONVERSION_SERVICE.get();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String expand(Object value) {
|
||||
return conversionService.convert(value, String.class);
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,7 +29,6 @@ import java.lang.reflect.Proxy;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
@@ -106,15 +105,6 @@ public class FeignClientTests {
|
||||
@Autowired
|
||||
HystrixClient hystrixClient;
|
||||
|
||||
protected enum Arg {
|
||||
A, B;
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return name().toLowerCase(Locale.ENGLISH);
|
||||
}
|
||||
}
|
||||
|
||||
@FeignClient(value = "localapp", configuration = TestClientConfig.class)
|
||||
protected interface TestClient {
|
||||
@RequestMapping(method = RequestMethod.GET, value = "/hello")
|
||||
@@ -140,9 +130,6 @@ public class FeignClientTests {
|
||||
|
||||
@RequestMapping(method = RequestMethod.HEAD, value = "/head")
|
||||
ResponseEntity head();
|
||||
|
||||
@RequestMapping(method = RequestMethod.GET, value = "/tostring")
|
||||
String getToString(@RequestParam("arg") Arg arg);
|
||||
}
|
||||
|
||||
public static class TestClientConfig {
|
||||
@@ -284,10 +271,6 @@ public class FeignClientTests {
|
||||
return ResponseEntity.status(HttpStatus.NOT_FOUND).body((String)null);
|
||||
}
|
||||
|
||||
@RequestMapping(method = RequestMethod.GET, value = "/tostring")
|
||||
String getToString(@RequestParam("arg") Arg arg) {
|
||||
return arg.toString();
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
new SpringApplicationBuilder(Application.class).properties(
|
||||
@@ -392,12 +375,6 @@ public class FeignClientTests {
|
||||
assertNull("response body was not null", response.getBody());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testConvertingExpander() {
|
||||
assertEquals(Arg.A.toString(), testClient.getToString(Arg.A));
|
||||
assertEquals(Arg.B.toString(), testClient.getToString(Arg.B));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHystrixFallbackWorks() {
|
||||
Hello hello = hystrixClient.fail();
|
||||
|
||||
Reference in New Issue
Block a user