diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java index a27c66d9..2250ee29 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java @@ -40,6 +40,7 @@ import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.AllNestedConditions; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -58,6 +59,7 @@ import org.springframework.cloud.openfeign.support.FeignHttpClientProperties; import org.springframework.cloud.openfeign.support.PageJacksonModule; import org.springframework.cloud.openfeign.support.SortJacksonModule; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.domain.Page; @@ -68,6 +70,7 @@ import org.springframework.data.domain.Sort; * @author Julien Roy * @author Grzegorz Poznachowski * @author Nikita Konev + * @author Tim Peeters */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(Feign.class) @@ -114,8 +117,7 @@ public class FeignAutoConfiguration { } @Configuration(proxyBeanMethods = false) - @ConditionalOnMissingClass({ "feign.hystrix.HystrixFeign", - "org.springframework.cloud.client.circuitbreaker.CircuitBreaker" }) + @Conditional(DefaultFeignTargeterConditions.class) protected static class DefaultFeignTargeterConfiguration { @Bean @@ -127,7 +129,10 @@ public class FeignAutoConfiguration { } @Configuration(proxyBeanMethods = false) + @Conditional(FeignCircuitBreakerDisabledConditions.class) @ConditionalOnClass(name = "feign.hystrix.HystrixFeign") + @ConditionalOnProperty(value = "feign.hystrix.enabled", havingValue = "true", + matchIfMissing = true) protected static class HystrixFeignTargeterConfiguration { @Bean @@ -140,15 +145,9 @@ public class FeignAutoConfiguration { @Configuration(proxyBeanMethods = false) @ConditionalOnClass(CircuitBreaker.class) - @ConditionalOnProperty("feign.circuitbreaker.enabled") + @ConditionalOnProperty(value = "feign.circuitbreaker.enabled", havingValue = "true") protected static class CircuitBreakerPresentFeignTargeterConfiguration { - @Bean - @ConditionalOnMissingBean(CircuitBreakerFactory.class) - public Targeter defaultFeignTargeter() { - return new DefaultTargeter(); - } - @Bean @ConditionalOnMissingBean @ConditionalOnBean(CircuitBreakerFactory.class) @@ -286,4 +285,22 @@ public class FeignAutoConfiguration { } + static class DefaultFeignTargeterConditions extends AllNestedConditions { + + DefaultFeignTargeterConditions() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Conditional(FeignCircuitBreakerDisabledConditions.class) + static class FeignCircuitBreakerDisabled { + + } + + @Conditional(HystrixDisabledConditions.class) + static class HystrixDisabled { + + } + + } + } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixDisabledConditions.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixDisabledConditions.java new file mode 100644 index 00000000..fc02e1f9 --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixDisabledConditions.java @@ -0,0 +1,42 @@ +/* + * Copyright 2013-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.openfeign; + +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; + +/** + * @author Tim Peeters + */ +class HystrixDisabledConditions extends AnyNestedCondition { + + HystrixDisabledConditions() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @ConditionalOnMissingClass("feign.hystrix.HystrixFeign") + static class HystrixFeignClassMissing { + + } + + @ConditionalOnProperty(value = "feign.hystrix.enabled", havingValue = "false") + static class HystrixFeignDisabled { + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java new file mode 100644 index 00000000..fa513dc7 --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java @@ -0,0 +1,107 @@ +/* + * Copyright 2013-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.openfeign; + +import feign.hystrix.HystrixFeign; +import org.assertj.core.api.Condition; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.FilteredClassLoader; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; +import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; +import org.springframework.context.ConfigurableApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +/** + * @author Tim Peeters + */ +class FeignAutoConfigurationTests { + + private final ApplicationContextRunner runner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(FeignAutoConfiguration.class)); + + @Test + void shouldInstantiateHystrixTargeterToMaintainBackwardsCompatibility() { + runner.run(ctx -> assertOnlyOneTargeterPresent(ctx, HystrixTargeter.class)); + } + + @Test + void shouldInstantiateHystrixTargeterWhenExplicitlyEnabled() { + runner.withPropertyValues("feign.hystrix.enabled=true") + .run(ctx -> assertOnlyOneTargeterPresent(ctx, HystrixTargeter.class)); + } + + @Test + void shouldInstantiateDefaultTargeterWhenHystrixIsDisabled() { + runner.withPropertyValues("feign.hystrix.enabled=false") + .run(ctx -> assertOnlyOneTargeterPresent(ctx, DefaultTargeter.class)); + } + + @Test + void shouldInstantiateDefaultTargeterWhenHystrixFeignClassIsMissing() { + runner.withPropertyValues("feign.hystrix.enabled=true") + .withClassLoader(new FilteredClassLoader(HystrixFeign.class)) + .run(ctx -> assertOnlyOneTargeterPresent(ctx, DefaultTargeter.class)); + } + + @Test + void shouldInstantiateDefaultTargeterWhenHystrixFeignAndCircuitBreakerClassesAreMissing() { + runner.withPropertyValues("feign.hystrix.enabled=true", + "feign.circuitbreaker.enabled=true") + .withClassLoader( + new FilteredClassLoader(HystrixFeign.class, CircuitBreaker.class)) + .run(ctx -> assertOnlyOneTargeterPresent(ctx, DefaultTargeter.class)); + } + + @Test + void shouldInstantiateDefaultTargeterWhenHystrixFeignClassIsMissingAndFeignCircuitBreakerIsDisabled() { + runner.withClassLoader(new FilteredClassLoader(HystrixFeign.class)) + .withPropertyValues("feign.circuitbreaker.enabled=false") + .run(ctx -> assertOnlyOneTargeterPresent(ctx, DefaultTargeter.class)); + } + + @Test + void shouldInstantiateFeignCircuitBreakerTargeterWhenEnabled() { + runner.withBean(CircuitBreakerFactory.class, + () -> mock(CircuitBreakerFactory.class)) + .withPropertyValues("feign.circuitbreaker.enabled=true") + .run(ctx -> assertOnlyOneTargeterPresent(ctx, + FeignCircuitBreakerTargeter.class)); + } + + @Test + void shouldInstantiateFeignCircuitBreakerTargeterWhenBothHystrixAndCircuitBreakerAreEnabled() { + runner.withBean(CircuitBreakerFactory.class, + () -> mock(CircuitBreakerFactory.class)) + .withPropertyValues("feign.hystrix.enabled=true", + "feign.circuitbreaker.enabled=true") + .run(ctx -> assertOnlyOneTargeterPresent(ctx, + FeignCircuitBreakerTargeter.class)); + } + + private void assertOnlyOneTargeterPresent(ConfigurableApplicationContext ctx, + Class beanClass) { + assertThat(ctx.getBeansOfType(Targeter.class)).hasSize(1) + .hasValueSatisfying(new Condition<>(beanClass::isInstance, String + .format("Targeter should be an instance of %s", beanClass))); + } + +}