diff --git a/spring-cloud-openfeign-core/pom.xml b/spring-cloud-openfeign-core/pom.xml index 7a326b27..3a7128b3 100644 --- a/spring-cloud-openfeign-core/pom.xml +++ b/spring-cloud-openfeign-core/pom.xml @@ -93,6 +93,10 @@ feign-core true + + io.github.openfeign.form + feign-form-spring + io.github.openfeign feign-slf4j @@ -168,6 +172,11 @@ spring-boot-autoconfigure-processor true + + org.springframework.boot + spring-boot-configuration-processor + true + org.springframework.boot spring-boot-starter-test diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java index d9925043..c2a488f5 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java @@ -62,19 +62,6 @@ class HystrixTargeter implements Targeter { Class fallbackFactoryClass) { FallbackFactory fallbackFactory = (FallbackFactory) getFromContext("fallbackFactory", feignClientName, context, fallbackFactoryClass, FallbackFactory.class); - /* We take a sample fallback from the fallback factory to check if it returns a fallback - that is compatible with the annotated feign interface. */ - Object exampleFallback = fallbackFactory.create(new RuntimeException()); - Assert.notNull(exampleFallback, - String.format( - "Incompatible fallbackFactory instance for feign client %s. Factory may not produce null!", - feignClientName)); - if (!target.type().isAssignableFrom(exampleFallback.getClass())) { - throw new IllegalStateException( - String.format( - "Incompatible fallbackFactory instance for feign client %s. Factory produces instances of '%s', but should produce instances of '%s'", - feignClientName, exampleFallback.getClass(), target.type())); - } return builder.target(target, fallbackFactory); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/LoadBalancerFeignClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/LoadBalancerFeignClient.java index ac18f7e1..4e210323 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/LoadBalancerFeignClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/LoadBalancerFeignClient.java @@ -97,7 +97,13 @@ public class LoadBalancerFeignClient implements Client { } static URI cleanUrl(String originalUrl, String host) { - return URI.create(originalUrl.replaceFirst(host, "")); + String newUrl = originalUrl.replaceFirst(host, ""); + StringBuffer buffer = new StringBuffer(newUrl); + if((newUrl.startsWith("https://") && newUrl.length() == 8) || + (newUrl.startsWith("http://") && newUrl.length() == 7)) { + buffer.append("/"); + } + return URI.create(buffer.toString()); } private FeignLoadBalancer lbClient(String clientName) { diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java index 0ee52016..fa4508ab 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java @@ -24,6 +24,7 @@ import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Collection; +import java.util.Objects; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,10 +36,12 @@ import org.springframework.http.MediaType; import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter; +import org.springframework.web.multipart.MultipartFile; import feign.RequestTemplate; import feign.codec.EncodeException; import feign.codec.Encoder; +import feign.form.spring.SpringFormEncoder; import static org.springframework.cloud.openfeign.support.FeignUtils.getHeaders; import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHeaders; @@ -51,6 +54,8 @@ public class SpringEncoder implements Encoder { private static final Log log = LogFactory.getLog(SpringEncoder.class); + private final SpringFormEncoder springFormEncoder = new SpringFormEncoder(); + private ObjectFactory messageConverters; public SpringEncoder(ObjectFactory messageConverters) { @@ -71,6 +76,18 @@ public class SpringEncoder implements Encoder { requestContentType = MediaType.valueOf(type); } + if (bodyType != null && bodyType.equals(MultipartFile.class)) { + if (Objects.equals(requestContentType, MediaType.MULTIPART_FORM_DATA)) { + springFormEncoder.encode(requestBody, bodyType, request); + return; + } else { + String message = "Content-Type \"" + MediaType.MULTIPART_FORM_DATA + + "\" not set for request body of type " + + requestBody.getClass().getSimpleName(); + throw new EncodeException(message); + } + } + for (HttpMessageConverter messageConverter : this.messageConverters .getObject().getConverters()) { if (messageConverter.canWrite(requestType, requestContentType)) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java index 9d4ca565..84dfe1c2 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java @@ -240,44 +240,4 @@ public class FeignClientValidationTests { } } - - @Test - public void testWrongFallbackFactoryGenericType() { - try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - WrongFallbackFactoryGenericTypeConfiguration.class)) { - this.expected.expectMessage("Incompatible fallbackFactory instance"); - assertNotNull(context.getBean(WrongFallbackFactoryGenericTypeConfiguration.Client.class)); - } - } - - @Configuration - @Import(FeignAutoConfiguration.class) - @EnableFeignClients(clients = WrongFallbackFactoryGenericTypeConfiguration.Client.class) - protected static class WrongFallbackFactoryGenericTypeConfiguration { - - @FeignClient(name = "foobar", url = "http://localhost", fallbackFactory = ClientFallback.class) - interface Client { - @RequestMapping(method = RequestMethod.GET, value = "/") - String get(); - } - - @Bean - ClientFallback dummy() { - return new ClientFallback(); - } - - class ClientFallback implements FallbackFactory { - - @Override - public String create(Throwable cause) { - return "tryinToTrickYa"; - } - } - - @Bean - public Feign.Builder feignBuilder() { - return HystrixFeign.builder(); - } - - } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java index ae82be9a..ab7fc108 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/FeignRibbonClientTests.java @@ -37,7 +37,9 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; /** * @author Dave Syer @@ -87,14 +89,24 @@ public class FeignRibbonClientTests { when(stats.getSingleServerStat(any(Server.class))).thenReturn(mock(ServerStats.class)); } + @Test + public void remoteRequestIsSentAtRoot() throws Exception { + Request request = new RequestTemplate().method("GET").append("http://foo") + .request(); + this.client.execute(request, new Options()); + RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); + } + @Test public void remoteRequestIsSent() throws Exception { Request request = new RequestTemplate().method("GET").append("http://foo/") .request(); this.client.execute(request, new Options()); RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); - /*FIXME verify(this.delegate).execute(argThat(matcher), - any(Options.class));*/ + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); } @Test @@ -103,8 +115,8 @@ public class FeignRibbonClientTests { .request(); this.client.execute(request, new Options()); RequestMatcher matcher = new RequestMatcher("https://foo.com:8000/"); - /*FIXME verify(this.delegate).execute(argThat(matcher), - any(Options.class));*/ + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); } private final static class RequestMatcher extends CustomMatcher { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java index f3672d94..fcc9b53e 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java @@ -40,9 +40,11 @@ import org.springframework.http.converter.AbstractGenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.http.converter.HttpMessageNotWritableException; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.multipart.MultipartFile; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -50,6 +52,7 @@ import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; import feign.RequestTemplate; +import feign.codec.EncodeException; /** * @author Spencer Gibb @@ -96,6 +99,31 @@ public class SpringEncoderTests { assertThat("request charset is not null", request.charset(), is(nullValue())); } + + @Test(expected = EncodeException.class) + public void testMultipartFile1() { + SpringEncoder encoder = this.context.getInstance("foo", SpringEncoder.class); + assertThat(encoder, is(notNullValue())); + RequestTemplate request = new RequestTemplate(); + + MultipartFile multipartFile = new MockMultipartFile("test_multipart_file", "hi".getBytes()); + encoder.encode(multipartFile, MultipartFile.class, request); + + assertThat("request charset is not null", request.charset(), is(nullValue())); + } + + @Test + public void testMultipartFile2() { + SpringEncoder encoder = this.context.getInstance("foo", SpringEncoder.class); + assertThat(encoder, is(notNullValue())); + RequestTemplate request = new RequestTemplate(); + request = request.header("Content-Type", MediaType.MULTIPART_FORM_DATA_VALUE); + + MultipartFile multipartFile = new MockMultipartFile("test_multipart_file", "hi".getBytes()); + encoder.encode(multipartFile, MultipartFile.class, request); + + assertThat("request charset is not null", request.charset(), is(nullValue())); + } class MediaTypeMatcher implements ArgumentMatcher { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java index 61cdf26c..d526c32a 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java @@ -70,6 +70,7 @@ import org.springframework.web.bind.annotation.RestController; import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; +import com.netflix.hystrix.exception.HystrixRuntimeException; import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; @@ -134,6 +135,12 @@ public class FeignClientTests { @Autowired private HystrixClientWithFallBackFactory hystrixClientWithFallBackFactory; + @Autowired + private InvalidTypeHystrixClientWithFallBackFactory invalidTypeHystrixClientWithFallBackFactory; + + @Autowired + private NullHystrixClientWithFallBackFactory nullHystrixClientWithFallBackFactory; + @Autowired @Qualifier("localapp3FeignClient") HystrixClient namedHystrixClient; @@ -285,6 +292,20 @@ public class FeignClientTests { Hello fail(); } + @FeignClient(name = "localapp6", fallbackFactory = InvalidTypeHystrixClientFallbackFactory.class) + protected interface InvalidTypeHystrixClientWithFallBackFactory { + + @RequestMapping(method = RequestMethod.GET, path = "/fail") + Hello fail(); + } + + @FeignClient(name = "localapp7", fallbackFactory = NullHystrixClientFallbackFactory.class) + protected interface NullHystrixClientWithFallBackFactory { + + @RequestMapping(method = RequestMethod.GET, path = "/fail") + Hello fail(); + } + static class HystrixClientFallbackFactory implements FallbackFactory { @Override @@ -299,6 +320,22 @@ public class FeignClientTests { } } + static class InvalidTypeHystrixClientFallbackFactory implements FallbackFactory { + + @Override + public String create(final Throwable cause) { + return "hello"; + } + } + + static class NullHystrixClientFallbackFactory implements FallbackFactory { + + @Override + public String create(final Throwable cause) { + return null; + } + } + static class HystrixClientFallback implements HystrixClient { @Override public Hello fail() { @@ -358,7 +395,8 @@ public class FeignClientTests { @RestController @EnableFeignClients(clients = { TestClientServiceId.class, TestClient.class, DecodingTestClient.class, HystrixClient.class, HystrixClientWithFallBackFactory.class, - HystrixSetterFactoryClient.class}, + HystrixSetterFactoryClient.class, InvalidTypeHystrixClientWithFallBackFactory.class, + NullHystrixClientWithFallBackFactory.class}, defaultConfiguration = TestDefaultFeignConfig.class) @RibbonClients({ @RibbonClient(name = "localapp", configuration = LocalRibbonClientConfiguration.class), @@ -366,7 +404,9 @@ public class FeignClientTests { @RibbonClient(name = "localapp2", configuration = LocalRibbonClientConfiguration.class), @RibbonClient(name = "localapp3", configuration = LocalRibbonClientConfiguration.class), @RibbonClient(name = "localapp4", configuration = LocalRibbonClientConfiguration.class), - @RibbonClient(name = "localapp5", configuration = LocalRibbonClientConfiguration.class) + @RibbonClient(name = "localapp5", configuration = LocalRibbonClientConfiguration.class), + @RibbonClient(name = "localapp6", configuration = LocalRibbonClientConfiguration.class), + @RibbonClient(name = "localapp7", configuration = LocalRibbonClientConfiguration.class) }) @Import(NoSecurityConfiguration.class) protected static class Application { @@ -382,6 +422,16 @@ public class FeignClientTests { return new HystrixClientFallbackFactory(); } + @Bean + public InvalidTypeHystrixClientFallbackFactory invalidTypeHystrixClientFallbackFactory() { + return new InvalidTypeHystrixClientFallbackFactory(); + } + + @Bean + public NullHystrixClientFallbackFactory nullHystrixClientFallbackFactory() { + return new NullHystrixClientFallbackFactory(); + } + @Bean FeignFormatterRegistrar feignFormatterRegistrar() { return new FeignFormatterRegistrar() { @@ -749,6 +799,16 @@ public class FeignClientTests { hello.getMessage().contains("500")); } + @Test(expected = HystrixRuntimeException.class) + public void testInvalidTypeHystrixFallbackFactory() throws Exception { + invalidTypeHystrixClientWithFallBackFactory.fail(); + } + + @Test(expected = HystrixRuntimeException.class) + public void testNullHystrixFallbackFactory() throws Exception { + nullHystrixClientWithFallBackFactory.fail(); + } + @Test public void namedFeignClientWorks() { assertNotNull("namedHystrixClient was null", this.namedHystrixClient); diff --git a/spring-cloud-openfeign-dependencies/pom.xml b/spring-cloud-openfeign-dependencies/pom.xml index 07519999..61a122bd 100644 --- a/spring-cloud-openfeign-dependencies/pom.xml +++ b/spring-cloud-openfeign-dependencies/pom.xml @@ -14,6 +14,7 @@ Spring Cloud OpenFeign Dependencies 9.7.0 + 3.3.0 @@ -32,6 +33,11 @@ feign-core ${feign.version} + + io.github.openfeign.form + feign-form-spring + ${feign-form.version} + io.github.openfeign feign-slf4j