From f00f7619960dd2a7ff23a054ebb6b9bb5cdb55a1 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 14 Jan 2016 16:19:50 -0700 Subject: [PATCH] Support feign hystrix fallbacks. Added @FeignClient(fallback). Renamed FeignClientFactory to FeignContext. fixes gh-762 --- .../main/asciidoc/spring-cloud-netflix.adoc | 23 ++++ .../netflix/feign/FeignAutoConfiguration.java | 8 +- .../cloud/netflix/feign/FeignClient.java | 6 + .../netflix/feign/FeignClientFactoryBean.java | 126 ++++++++++++++---- .../feign/FeignClientsConfiguration.java | 18 +-- .../netflix/feign/FeignClientsRegistrar.java | 1 + ...gnClientFactory.java => FeignContext.java} | 4 +- .../feign/support/FallbackCommand.java | 55 ++++++++ .../feign/EnableFeignClientsTests.java | 12 +- .../feign/FeignClientFactoryTests.java | 12 +- .../FeignClientOverrideDefaultsTests.java | 42 +++--- .../netflix/feign/SpringDecoderTests.java | 4 +- .../invalid/FeignClientValidationTests.java | 68 +++++++++- .../netflix/feign/valid/FeignClientTests.java | 97 +++++++++++++- 14 files changed, 392 insertions(+), 84 deletions(-) rename spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/{FeignClientFactory.java => FeignContext.java} (89%) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FallbackCommand.java diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 5afae38d..4ec76e77 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -860,6 +860,29 @@ public class FooConfiguration { } ---- +[[spring-cloud-feign-hystrix-fallback]] +=== Feign Hystrix Fallbacks + +Hystrix supports the notion of a fallback: a default code path that is executed when they circuit is open or there is an error. To enable fallbacks for a given `@FeignClient` set the `fallback` attribute to the class name that implements the fallback. + +[source,java,indent=0] +---- +@FeignClient(name = "hello", fallback = HystrixClientFallback.class) +protected interface HystrixClient { + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello iFailSometimes(); +} + +static class HystrixClientFallback implements HystrixClient { + @Override + public Hello iFailSometimes() { + return new Hello("fallback"); + } +} +---- + +WARNING: There is a limitation with the implementation of fallbacks in Feign and how Hystrix fallbacks work. Fallbacks are currently not supported for methods that return `com.netflix.hystrix.HystrixCommand` and `rx.Observable`. + [[spring-cloud-feign-inheritance]] === Feign Inheritance Support diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index d8c470ff..1ec41430 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -44,9 +44,9 @@ public class FeignAutoConfiguration { } @Bean - public FeignClientFactory feignClientFactory() { - FeignClientFactory factory = new FeignClientFactory(); - factory.setConfigurations(this.configurations); - return factory; + public FeignContext feignContext() { + FeignContext context = new FeignContext(); + context.setConfigurations(this.configurations); + return context; } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java index 422fc4f9..c82cb500 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java @@ -76,4 +76,10 @@ public @interface FeignClient { * @see FeignClientsConfiguration for the defaults */ Class[] configuration() default {}; + + /** + * Fallback class for the specified Feign client interface. The fallback class must + * implement the interface annotated by this annotation and be a valid spring bean. + */ + Class fallback() default void.class; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java index c024f875..eac0abb2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java @@ -24,6 +24,7 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import feign.Client; @@ -33,7 +34,6 @@ import feign.Logger; import feign.Request; import feign.RequestInterceptor; import feign.Retryer; -import feign.Target; import feign.Target.HardCodedTarget; import feign.codec.Decoder; import feign.codec.Encoder; @@ -47,7 +47,22 @@ import lombok.EqualsAndHashCode; */ @Data @EqualsAndHashCode(callSuper = false) -class FeignClientFactoryBean implements FactoryBean, InitializingBean, ApplicationContextAware { +class FeignClientFactoryBean implements FactoryBean, InitializingBean, + ApplicationContextAware { + + private static final Targeter targeter; + + static { + Targeter targeterToUse; + if (ClassUtils.isPresent("feign.hystrix.HystrixFeign", + FeignClientFactoryBean.class.getClassLoader())) { + targeterToUse = new HystrixTargeter(); + } + else { + targeterToUse = new DefaultTargeter(); + } + targeter = targeterToUse; + } private Class type; @@ -57,7 +72,9 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, A private boolean decode404; - private ApplicationContext context; + private ApplicationContext applicationContext; + + private Class fallback = void.class; @Override public void afterPropertiesSet() throws Exception { @@ -66,43 +83,44 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, A @Override public void setApplicationContext(ApplicationContext context) throws BeansException { - this.context = context; + this.applicationContext = context; } - protected Feign.Builder feign(FeignClientFactory factory) { - Logger logger = getOptional(factory, Logger.class); + protected Feign.Builder feign(FeignContext context) { + Logger logger = getOptional(context, Logger.class); if (logger == null) { logger = new Slf4jLogger(this.type); } // @formatter:off - Feign.Builder builder = get(factory, Feign.Builder.class) + Feign.Builder builder = get(context, Feign.Builder.class) // required values .logger(logger) - .encoder(get(factory, Encoder.class)) - .decoder(get(factory, Decoder.class)) - .contract(get(factory, Contract.class)); + .encoder(get(context, Encoder.class)) + .decoder(get(context, Decoder.class)) + .contract(get(context, Contract.class)); // @formatter:on // optional values - Logger.Level level = getOptional(factory, Logger.Level.class); + Logger.Level level = getOptional(context, Logger.Level.class); if (level != null) { builder.logLevel(level); } - Retryer retryer = getOptional(factory, Retryer.class); + Retryer retryer = getOptional(context, Retryer.class); if (retryer != null) { builder.retryer(retryer); } - ErrorDecoder errorDecoder = getOptional(factory, ErrorDecoder.class); + ErrorDecoder errorDecoder = getOptional(context, ErrorDecoder.class); if (errorDecoder != null) { builder.errorDecoder(errorDecoder); } - Request.Options options = getOptional(factory, Request.Options.class); + Request.Options options = getOptional(context, Request.Options.class); if (options != null) { builder.options(options); } - Map requestInterceptors = factory.getInstances(this.name, RequestInterceptor.class); + Map requestInterceptors = context.getInstances( + this.name, RequestInterceptor.class); if (requestInterceptors != null) { builder.requestInterceptors(requestInterceptors.values()); } @@ -114,44 +132,52 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, A return builder; } - protected T get(FeignClientFactory factory, Class type) { - T instance = factory.getInstance(this.name, type); + protected T get(FeignContext context, Class type) { + T instance = context.getInstance(this.name, type); if (instance == null) { - throw new IllegalStateException("No bean found of type " + type + " for " + this.name); + throw new IllegalStateException("No bean found of type " + type + " for " + + this.name); } return instance; } - protected T getOptional(FeignClientFactory factory, Class type) { - return factory.getInstance(this.name, type); + protected T getOptional(FeignContext context, Class type) { + return context.getInstance(this.name, type); } - protected T loadBalance(Feign.Builder builder, FeignClientFactory factory, Target target) { - Client client = getOptional(factory, Client.class); + protected T loadBalance(Feign.Builder builder, FeignContext context, + HardCodedTarget target) { + Client client = getOptional(context, Client.class); if (client != null) { - return builder.client(client).target(target); + builder.client(client); + return targeter.target(this, builder, context, target); } - throw new IllegalStateException("No Feign Client for loadBalancing defined. Did you forget to include spring-cloud-starter-ribbon?"); + throw new IllegalStateException( + "No Feign Client for loadBalancing defined. Did you forget to include spring-cloud-starter-ribbon?"); } @Override public Object getObject() throws Exception { - FeignClientFactory factory = context.getBean(FeignClientFactory.class); + FeignContext context = applicationContext.getBean(FeignContext.class); + Feign.Builder builder = feign(context); if (!StringUtils.hasText(this.url)) { String url; if (!this.name.startsWith("http")) { url = "http://" + this.name; - } else { + } + else { url = this.name; } - return loadBalance(feign(factory), factory, new HardCodedTarget<>(this.type, this.name, url)); + return loadBalance(builder, context, new HardCodedTarget<>(this.type, + this.name, url)); } if (StringUtils.hasText(this.url) && !this.url.startsWith("http")) { this.url = "http://" + this.url; } - return feign(factory).target(new HardCodedTarget<>(this.type, this.name, this.url)); + return targeter.target(this, builder, context, new HardCodedTarget<>( + this.type, this.name, this.url)); } @Override @@ -164,4 +190,48 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, A return true; } + interface Targeter { + T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + HardCodedTarget target); + } + + static class DefaultTargeter implements Targeter { + + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + HardCodedTarget target) { + return feign.target(target); + } + } + + @SuppressWarnings("unchecked") + static class HystrixTargeter implements Targeter { + + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, FeignContext context, + HardCodedTarget target) { + if (factory.fallback == void.class + || !(feign instanceof feign.hystrix.HystrixFeign.Builder)) { + return feign.target(target); + } + + Object fallbackInstance = context.getInstance(factory.name, factory.fallback); + if (fallbackInstance == null) { + throw new IllegalStateException(String.format( + "No fallback instance of type %s found for feign client %s", + factory.fallback, factory.name)); + } + + if (!target.type().isAssignableFrom(factory.fallback)) { + throw new IllegalStateException( + String.format( + "Incompatible fallback instance. Fallback of type %s is not assignable to %s for feign client %s", + factory.fallback, target.type(), factory.name)); + } + + feign.hystrix.HystrixFeign.Builder builder = (feign.hystrix.HystrixFeign.Builder) feign; + return builder.target(target, (T) fallbackInstance); + } + } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java index 87ebb78f..0f065be0 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java @@ -74,19 +74,21 @@ public class FeignClientsConfiguration { return new SpringMvcContract(parameterProcessors); } - @Bean - @Scope("prototype") - @ConditionalOnMissingBean - @ConditionalOnClass(HystrixCommand.class) - @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) - public Feign.Builder feignHystrixBuilder() { - return HystrixFeign.builder(); + @Configuration + @ConditionalOnClass({ HystrixCommand.class, HystrixFeign.class }) + protected static class HystrixFeignConfiguration { + @Bean + @Scope("prototype") + @ConditionalOnMissingBean + @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) + public Feign.Builder feignHystrixBuilder() { + return HystrixFeign.builder(); + } } @Bean @Scope("prototype") @ConditionalOnMissingBean - @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = false, havingValue = "false") public Feign.Builder feignBuilder() { return Feign.builder(); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java index f1cdf717..9a234b89 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java @@ -174,6 +174,7 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, definition.addPropertyValue("name", getServiceId(attributes)); definition.addPropertyValue("type", className); definition.addPropertyValue("decode404", attributes.get("decode404")); + definition.addPropertyValue("fallback", attributes.get("fallback")); definition.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE); String beanName = StringUtils diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignContext.java similarity index 89% rename from spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactory.java rename to spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignContext.java index 7aa2e777..4361fa83 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignContext.java @@ -26,9 +26,9 @@ import org.springframework.cloud.context.named.NamedContextFactory; * @author Spencer Gibb * @author Dave Syer */ -public class FeignClientFactory extends NamedContextFactory { +public class FeignContext extends NamedContextFactory { - public FeignClientFactory() { + public FeignContext() { super(FeignClientsConfiguration.class, "feign", "feign.client.name"); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FallbackCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FallbackCommand.java new file mode 100644 index 00000000..9bc03080 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FallbackCommand.java @@ -0,0 +1,55 @@ +package org.springframework.cloud.netflix.feign.support; + +import com.netflix.hystrix.HystrixCommand; +import com.netflix.hystrix.HystrixCommandGroupKey; +import com.netflix.hystrix.HystrixThreadPoolKey; + +/** + * Convenience class for implementing feign fallbacks that return {@link HystrixCommand}. + * Also useful for return types of {@link rx.Observable} and {@link java.util.concurrent.Future}. + * For those return types, just call {@link FallbackCommand#observe()} or {@link FallbackCommand#queue()} respectively. + * @author Spencer Gibb + */ +public class FallbackCommand extends HystrixCommand { + + private T result; + + public FallbackCommand(T result) { + this(result, "fallback"); + } + + protected FallbackCommand(T result, String groupname) { + super(HystrixCommandGroupKey.Factory.asKey(groupname)); + this.result = result; + } + + public FallbackCommand(T result, HystrixCommandGroupKey group) { + super(group); + this.result = result; + } + + public FallbackCommand(T result, HystrixCommandGroupKey group, int executionIsolationThreadTimeoutInMilliseconds) { + super(group, executionIsolationThreadTimeoutInMilliseconds); + this.result = result; + } + + public FallbackCommand(T result, HystrixCommandGroupKey group, HystrixThreadPoolKey threadPool) { + super(group, threadPool); + this.result = result; + } + + public FallbackCommand(T result, HystrixCommandGroupKey group, HystrixThreadPoolKey threadPool, int executionIsolationThreadTimeoutInMilliseconds) { + super(group, threadPool, executionIsolationThreadTimeoutInMilliseconds); + this.result = result; + } + + public FallbackCommand(T result, Setter setter) { + super(setter); + this.result = result; + } + + @Override + protected T run() throws Exception { + return this.result; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/EnableFeignClientsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/EnableFeignClientsTests.java index 45ab4536..56eb2b1f 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/EnableFeignClientsTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/EnableFeignClientsTests.java @@ -47,34 +47,34 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; public class EnableFeignClientsTests { @Autowired - private FeignClientFactory factory; + private FeignContext feignContext; @Autowired private ApplicationContext context; @Test public void decoderDefaultCorrect() { - ResponseEntityDecoder.class.cast(this.factory.getInstance("foo", Decoder.class)); + ResponseEntityDecoder.class.cast(this.feignContext.getInstance("foo", Decoder.class)); } @Test public void encoderDefaultCorrect() { - SpringEncoder.class.cast(this.factory.getInstance("foo", Encoder.class)); + SpringEncoder.class.cast(this.feignContext.getInstance("foo", Encoder.class)); } @Test public void loggerDefaultCorrect() { - Slf4jLogger.class.cast(this.factory.getInstance("foo", Logger.class)); + Slf4jLogger.class.cast(this.feignContext.getInstance("foo", Logger.class)); } @Test public void contractDefaultCorrect() { - SpringMvcContract.class.cast(this.factory.getInstance("foo", Contract.class)); + SpringMvcContract.class.cast(this.feignContext.getInstance("foo", Contract.class)); } @Test public void builderDefaultCorrect() { - HystrixFeign.Builder.class.cast(this.factory.getInstance("foo", Feign.Builder.class)); + HystrixFeign.Builder.class.cast(this.feignContext.getInstance("foo", Feign.Builder.class)); } @Configuration diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientFactoryTests.java index 225f07f3..30921294 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientFactoryTests.java @@ -34,18 +34,18 @@ public class FeignClientFactoryTests { public void testChildContexts() { AnnotationConfigApplicationContext parent = new AnnotationConfigApplicationContext(); parent.refresh(); - FeignClientFactory factory = new FeignClientFactory(); - factory.setApplicationContext(parent); - factory.setConfigurations(Arrays.asList(getSpec("foo", FooConfig.class), + FeignContext context = new FeignContext(); + context.setApplicationContext(parent); + context.setConfigurations(Arrays.asList(getSpec("foo", FooConfig.class), getSpec("bar", BarConfig.class))); - Foo foo = factory.getInstance("foo", Foo.class); + Foo foo = context.getInstance("foo", Foo.class); assertThat("foo was null", foo, is(notNullValue())); - Bar bar = factory.getInstance("bar", Bar.class); + Bar bar = context.getInstance("bar", Bar.class); assertThat("bar was null", bar, is(notNullValue())); - Bar foobar = factory.getInstance("foo", Bar.class); + Bar foobar = context.getInstance("foo", Bar.class); assertThat("bar was not null", foobar, is(nullValue())); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java index a55790d9..b879cbf8 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignClientOverrideDefaultsTests.java @@ -58,63 +58,63 @@ import feign.slf4j.Slf4jLogger; public class FeignClientOverrideDefaultsTests { @Autowired - private FeignClientFactory factory; + private FeignContext context; @Test public void overrideDecoder() { - Decoder.Default.class.cast(this.factory.getInstance("foo", Decoder.class)); - ResponseEntityDecoder.class.cast(this.factory.getInstance("bar", Decoder.class)); + Decoder.Default.class.cast(this.context.getInstance("foo", Decoder.class)); + ResponseEntityDecoder.class.cast(this.context.getInstance("bar", Decoder.class)); } @Test public void overrideEncoder() { - Encoder.Default.class.cast(this.factory.getInstance("foo", Encoder.class)); - SpringEncoder.class.cast(this.factory.getInstance("bar", Encoder.class)); + Encoder.Default.class.cast(this.context.getInstance("foo", Encoder.class)); + SpringEncoder.class.cast(this.context.getInstance("bar", Encoder.class)); } @Test public void overrideLogger() { - Logger.JavaLogger.class.cast(this.factory.getInstance("foo", Logger.class)); - Slf4jLogger.class.cast(this.factory.getInstance("bar", Logger.class)); + Logger.JavaLogger.class.cast(this.context.getInstance("foo", Logger.class)); + Slf4jLogger.class.cast(this.context.getInstance("bar", Logger.class)); } @Test public void overrideContract() { - Contract.Default.class.cast(this.factory.getInstance("foo", Contract.class)); - SpringMvcContract.class.cast(this.factory.getInstance("bar", Contract.class)); + Contract.Default.class.cast(this.context.getInstance("foo", Contract.class)); + SpringMvcContract.class.cast(this.context.getInstance("bar", Contract.class)); } @Test public void overrideLoggerLevel() { - assertNull(this.factory.getInstance("foo", Logger.Level.class)); + assertNull(this.context.getInstance("foo", Logger.Level.class)); assertEquals(Logger.Level.HEADERS, - this.factory.getInstance("bar", Logger.Level.class)); + this.context.getInstance("bar", Logger.Level.class)); } @Test public void overrideRetryer() { - assertNull(this.factory.getInstance("foo", Retryer.class)); - Retryer.Default.class.cast(this.factory.getInstance("bar", Retryer.class)); + assertNull(this.context.getInstance("foo", Retryer.class)); + Retryer.Default.class.cast(this.context.getInstance("bar", Retryer.class)); } @Test public void overrideErrorDecoder() { - assertNull(this.factory.getInstance("foo", ErrorDecoder.class)); + assertNull(this.context.getInstance("foo", ErrorDecoder.class)); ErrorDecoder.Default.class - .cast(this.factory.getInstance("bar", ErrorDecoder.class)); + .cast(this.context.getInstance("bar", ErrorDecoder.class)); } @Test public void overrideBuilder() { - Feign.Builder.class.cast(this.factory.getInstance("foo", Feign.Builder.class)); + Feign.Builder.class.cast(this.context.getInstance("foo", Feign.Builder.class)); HystrixFeign.Builder.class - .cast(this.factory.getInstance("bar", Feign.Builder.class)); + .cast(this.context.getInstance("bar", Feign.Builder.class)); } @Test public void overrideRequestOptions() { - assertNull(this.factory.getInstance("foo", Request.Options.class)); - Request.Options options = this.factory.getInstance("bar", Request.Options.class); + assertNull(this.context.getInstance("foo", Request.Options.class)); + Request.Options options = this.context.getInstance("bar", Request.Options.class); assertEquals(1, options.connectTimeoutMillis()); assertEquals(1, options.readTimeoutMillis()); } @@ -122,9 +122,9 @@ public class FeignClientOverrideDefaultsTests { @Test public void addRequestInterceptor() { assertEquals(1, - this.factory.getInstances("foo", RequestInterceptor.class).size()); + this.context.getInstances("foo", RequestInterceptor.class).size()); assertEquals(2, - this.factory.getInstances("bar", RequestInterceptor.class).size()); + this.context.getInstances("bar", RequestInterceptor.class).size()); } @Configuration diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java index b718b3e3..e1c353d6 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java @@ -57,7 +57,7 @@ import static org.junit.Assert.assertNull; public class SpringDecoderTests extends FeignClientFactoryBean { @Autowired - FeignClientFactory factory; + FeignContext context; @Value("${local.server.port}") private int port = 0; @@ -73,7 +73,7 @@ public class SpringDecoderTests extends FeignClientFactoryBean { public TestClient testClient(boolean decode404) { setType(this.getClass()); setDecode404(decode404); - return feign(factory).target(TestClient.class, "http://localhost:" + this.port); + return feign(context).target(TestClient.class, "http://localhost:" + this.port); } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java index 5aeea0ff..4ec6bac3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java @@ -23,6 +23,7 @@ import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.FeignClient; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.web.bind.annotation.RequestMapping; @@ -39,18 +40,18 @@ public class FeignClientValidationTests { public ExpectedException expected = ExpectedException.none(); @Test - public void invalid() { + public void testNotLegalHostname() { this.expected.expectMessage("not legal hostname (foo_bar)"); AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( - BadConfiguration.class); - assertNotNull(context.getBean(BadConfiguration.Client.class)); + BadHostnameConfiguration.class); + assertNotNull(context.getBean(BadHostnameConfiguration.Client.class)); context.close(); } @Configuration @Import(FeignAutoConfiguration.class) - @EnableFeignClients(clients = BadConfiguration.Client.class) - protected static class BadConfiguration { + @EnableFeignClients(clients = BadHostnameConfiguration.Client.class) + protected static class BadHostnameConfiguration { @FeignClient("foo_bar") interface Client { @@ -60,4 +61,61 @@ public class FeignClientValidationTests { } + @Test + public void testMissingFallback() { + this.expected.expectMessage("No fallback instance of type"); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + MissingFallbackConfiguration.class); + assertNotNull(context.getBean(MissingFallbackConfiguration.Client.class)); + context.close(); + } + + @Configuration + @Import(FeignAutoConfiguration.class) + @EnableFeignClients(clients = MissingFallbackConfiguration.Client.class) + protected static class MissingFallbackConfiguration { + + @FeignClient(name = "foobar", url = "http://localhost", fallback = ClientFallback.class) + interface Client { + @RequestMapping(method = RequestMethod.GET, value = "/") + String get(); + } + + class ClientFallback implements Client { + @Override + public String get() { + return null; + } + } + + } + + @Test + public void testWrongFallbackType() { + this.expected.expectMessage("Incompatible fallback instance"); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + WrongFallbackTypeConfiguration.class); + assertNotNull(context.getBean(WrongFallbackTypeConfiguration.Client.class)); + context.close(); + } + + @Configuration + @Import(FeignAutoConfiguration.class) + @EnableFeignClients(clients = WrongFallbackTypeConfiguration.Client.class) + protected static class WrongFallbackTypeConfiguration { + + @FeignClient(name = "foobar", url = "http://localhost", fallback = Dummy.class) + interface Client { + @RequestMapping(method = RequestMethod.GET, value = "/") + String get(); + } + + @Bean + Dummy dummy() { + return new Dummy(); + } + + class Dummy { } + + } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java index 9c382685..d7cd5af3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java @@ -29,11 +29,14 @@ import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -43,6 +46,7 @@ import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.support.FallbackCommand; import org.springframework.cloud.netflix.feign.FeignClient; import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; import org.springframework.cloud.netflix.ribbon.RibbonClient; @@ -68,6 +72,7 @@ import feign.Client; import feign.Logger; import feign.RequestInterceptor; import feign.RequestTemplate; +import rx.Observable; /** * @author Spencer Gibb @@ -97,6 +102,9 @@ public class FeignClientTests { @Autowired private Client feignClient; + @Autowired + HystrixClient hystrixClient; + @FeignClient(value = "localapp", configuration = TestClientConfig.class) protected interface TestClient { @RequestMapping(method = RequestMethod.GET, value = "/hello") @@ -159,15 +167,53 @@ public class FeignClientTests { ResponseEntity notFound(); } + @FeignClient(name = "localapp3", fallback = HystrixClientFallback.class) + protected interface HystrixClient { + @RequestMapping(method = RequestMethod.GET, value = "/fail") + Hello fail(); + + @RequestMapping(method = RequestMethod.GET, value = "/fail") + HystrixCommand failCommand(); + + @RequestMapping(method = RequestMethod.GET, value = "/fail") + Observable failObservable(); + + @RequestMapping(method = RequestMethod.GET, value = "/fail") + Future failFuture(); + } + + static class HystrixClientFallback implements HystrixClient { + @Override + public Hello fail() { + return new Hello("fallback"); + } + + @Override + public HystrixCommand failCommand() { + return new FallbackCommand<>(new Hello("fallbackcommand")); + } + + @Override + public Observable failObservable() { + return new FallbackCommand<>(new Hello("fallbackobservable")).observe(); + } + + @Override + public Future failFuture() { + return new FallbackCommand<>(new Hello("fallbackfuture")).queue(); + } + } + @Configuration @EnableAutoConfiguration @RestController - @EnableFeignClients(clients = {TestClientServiceId.class, TestClient.class, DecodingTestClient.class}, + @EnableFeignClients(clients = {TestClientServiceId.class, TestClient.class, DecodingTestClient.class, HystrixClient.class}, defaultConfiguration = TestDefaultFeignConfig.class) @RibbonClients({ @RibbonClient(name = "localapp", configuration = LocalRibbonClientConfiguration.class), @RibbonClient(name = "localapp1", configuration = LocalRibbonClientConfiguration.class), - @RibbonClient(name = "localapp2", configuration = LocalRibbonClientConfiguration.class) + @RibbonClient(name = "localapp2", configuration = LocalRibbonClientConfiguration.class), + @RibbonClient(name = "localapp3", configuration = LocalRibbonClientConfiguration.class), }) protected static class Application { @@ -215,11 +261,17 @@ public class FeignClientTests { return ResponseEntity.ok().build(); } + @RequestMapping(method = RequestMethod.GET, value = "/fail") + String fail() { + throw new RuntimeException("always fails"); + } + @RequestMapping(method = RequestMethod.GET, value = "/notFound") ResponseEntity notFound() { return ResponseEntity.status(HttpStatus.NOT_FOUND).body((String)null); } + public static void main(String[] args) { new SpringApplicationBuilder(Application.class).properties( "spring.application.name=feignclienttest", @@ -323,6 +375,42 @@ public class FeignClientTests { assertNull("response body was not null", response.getBody()); } + @Test + public void testHystrixFallbackWorks() { + Hello hello = hystrixClient.fail(); + assertNotNull("hello was null", hello); + assertEquals("message was wrong", "fallback", hello.getMessage()); + } + + @Test + @Ignore("Until HystrixCommand works in fallback") + public void testHystrixFallbackCommand() { + HystrixCommand command = hystrixClient.failCommand(); + assertNotNull("command was null", command); + Hello hello = command.execute(); + assertNotNull("hello was null", hello); + assertEquals("message was wrong", "fallbackcommand", hello.getMessage()); + } + + @Test + @Ignore("Until Observable works in fallback") + public void testHystrixFallbackObservable() { + Observable observable = hystrixClient.failObservable(); + assertNotNull("observable was null", observable); + Hello hello = observable.toBlocking().first(); + assertNotNull("hello was null", hello); + assertEquals("message was wrong", "fallbackobservable", hello.getMessage()); + } + + @Test + public void testHystrixFallbackFuture() throws Exception { + Future future = hystrixClient.failFuture(); + assertNotNull("future was null", future); + Hello hello = future.get(1, TimeUnit.SECONDS); + assertNotNull("hello was null", hello); + assertEquals("message was wrong", "fallbackfuture", hello.getMessage()); + } + @Data @AllArgsConstructor @NoArgsConstructor @@ -336,6 +424,11 @@ public class FeignClientTests { Logger.Level feignLoggerLevel() { return Logger.Level.FULL; } + + @Bean + public HystrixClientFallback hystrixClientFallback() { + return new HystrixClientFallback(); + } } // Load balancer with fixed server list for "local" pointing to localhost