Fixes #482 No bean found of type Targeter. (#483)

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 {
+
+		}
+
+	}
+
 }

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 {
+
+	}
+
+}

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)));
+	}
+
+}