Browse Source

Remove code which was calling FallbackFactory with RuntimeException. Fixes #41 (#58)

pull/64/head
Ryan Baxter 7 years ago committed by GitHub
parent
commit
06ccb9d1e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 13
      spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java
  2. 40
      spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java
  3. 64
      spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/valid/FeignClientTests.java

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

@ -62,19 +62,6 @@ class HystrixTargeter implements Targeter { @@ -62,19 +62,6 @@ class HystrixTargeter implements Targeter {
Class<?> fallbackFactoryClass) {
FallbackFactory<? extends T> fallbackFactory = (FallbackFactory<? extends T>)
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);
}

40
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/invalid/FeignClientValidationTests.java

@ -240,44 +240,4 @@ public class FeignClientValidationTests { @@ -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<String> {
@Override
public String create(Throwable cause) {
return "tryinToTrickYa";
}
}
@Bean
public Feign.Builder feignBuilder() {
return HystrixFeign.builder();
}
}
}

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

@ -66,6 +66,7 @@ import org.springframework.web.bind.annotation.RestController; @@ -66,6 +66,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;
@ -129,6 +130,12 @@ public class FeignClientTests { @@ -129,6 +130,12 @@ public class FeignClientTests {
@Autowired
private HystrixClientWithFallBackFactory hystrixClientWithFallBackFactory;
@Autowired
private InvalidTypeHystrixClientWithFallBackFactory invalidTypeHystrixClientWithFallBackFactory;
@Autowired
private NullHystrixClientWithFallBackFactory nullHystrixClientWithFallBackFactory;
@Autowired
@Qualifier("localapp3FeignClient")
HystrixClient namedHystrixClient;
@ -275,6 +282,20 @@ public class FeignClientTests { @@ -275,6 +282,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<HystrixClientWithFallBackFactory> {
@Override
@ -289,6 +310,22 @@ public class FeignClientTests { @@ -289,6 +310,22 @@ public class FeignClientTests {
}
}
static class InvalidTypeHystrixClientFallbackFactory implements FallbackFactory<String> {
@Override
public String create(final Throwable cause) {
return "hello";
}
}
static class NullHystrixClientFallbackFactory implements FallbackFactory<String> {
@Override
public String create(final Throwable cause) {
return null;
}
}
static class HystrixClientFallback implements HystrixClient {
@Override
public Hello fail() {
@ -348,7 +385,8 @@ public class FeignClientTests { @@ -348,7 +385,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),
@ -356,7 +394,9 @@ public class FeignClientTests { @@ -356,7 +394,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)
})
protected static class Application {
@ -371,6 +411,16 @@ public class FeignClientTests { @@ -371,6 +411,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() {
@ -722,6 +772,16 @@ public class FeignClientTests { @@ -722,6 +772,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);

Loading…
Cancel
Save