Use CircuitBreaker even when no fallback. (#466)

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

@@ -16,12 +16,7 @@
 
 package org.springframework.cloud.openfeign;
 
-import java.lang.reflect.InvocationHandler;
-import java.lang.reflect.Method;
-import java.util.Map;
-
 import feign.Feign;
-import feign.InvocationHandlerFactory;
 import feign.Target;
 
 import org.springframework.cloud.client.circuitbreaker.CircuitBreaker;
@@ -76,15 +71,16 @@ public final class FeignCircuitBreaker {
 			return build(fallbackFactory).newInstance(target);
 		}
 
+		@Override
+		public <T> T target(Target<T> target) {
+			return build(null).newInstance(target);
+		}
+
 		public Feign build(final FallbackFactory<?> nullableFallbackFactory) {
-			super.invocationHandlerFactory(new InvocationHandlerFactory() {
-				@Override
-				public InvocationHandler create(Target target,
-						Map<Method, MethodHandler> dispatch) {
-					return new FeignCircuitBreakerInvocationHandler(circuitBreakerFactory,
-							feignClientName, target, dispatch, nullableFallbackFactory);
-				}
-			});
+			super.invocationHandlerFactory(
+					(target, dispatch) -> new FeignCircuitBreakerInvocationHandler(
+							circuitBreakerFactory, feignClientName, target, dispatch,
+							nullableFallbackFactory));
 			return super.build();
 		}
 

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2020 the original author or authors.
+ * Copyright 2013-2021 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.
@@ -16,10 +16,7 @@
 
 package org.springframework.cloud.openfeign.circuitbreaker;
 
-import java.util.Objects;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
-import java.util.function.Supplier;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -59,12 +56,12 @@ import static org.assertj.core.api.Assertions.assertThat;
  * @author Spencer Gibb
  */
 @RunWith(SpringJUnit4ClassRunner.class)
-@SpringBootTest(classes = CirciutBreakerTests.Application.class,
+@SpringBootTest(classes = CircuitBreakerTests.Application.class,
 		webEnvironment = WebEnvironment.DEFINED_PORT,
 		value = { "spring.application.name=springcircuittest", "spring.jmx.enabled=false",
 				"feign.circuitbreaker.enabled=true" })
 @DirtiesContext
-public class CirciutBreakerTests {
+public class CircuitBreakerTests {
 
 	@Autowired
 	MyCircuitBreaker myCircuitBreaker;
@@ -193,44 +190,6 @@ public class CirciutBreakerTests {
 	}
 	// end::client_with_fallback_factory[]
 
-	public static class Hello {
-
-		private String message;
-
-		public Hello() {
-		}
-
-		public Hello(String message) {
-			this.message = message;
-		}
-
-		public String getMessage() {
-			return this.message;
-		}
-
-		public void setMessage(String message) {
-			this.message = message;
-		}
-
-		@Override
-		public boolean equals(Object o) {
-			if (this == o) {
-				return true;
-			}
-			if (o == null || getClass() != o.getClass()) {
-				return false;
-			}
-			Hello that = (Hello) o;
-			return Objects.equals(this.message, that.message);
-		}
-
-		@Override
-		public int hashCode() {
-			return Objects.hash(this.message);
-		}
-
-	}
-
 	@Configuration(proxyBeanMethods = false)
 	@EnableAutoConfiguration
 	@RestController
@@ -288,30 +247,4 @@ public class CirciutBreakerTests {
 
 	}
 
-	static class MyCircuitBreaker implements CircuitBreaker {
-
-		AtomicBoolean runWasCalled = new AtomicBoolean();
-
-		@Override
-		public <T> T run(Supplier<T> toRun) {
-			this.runWasCalled.set(true);
-			return toRun.get();
-		}
-
-		@Override
-		public <T> T run(Supplier<T> toRun, Function<Throwable, T> fallback) {
-			try {
-				return run(toRun);
-			}
-			catch (Throwable throwable) {
-				return fallback.apply(throwable);
-			}
-		}
-
-		public void clear() {
-			this.runWasCalled.set(false);
-		}
-
-	}
-
 }

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerWithNoFallbackTests.java

@@ -0,0 +1,159 @@
+/*
+ * Copyright 2013-2021 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.circuitbreaker;
+
+import java.util.function.Function;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.cloud.client.circuitbreaker.CircuitBreaker;
+import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory;
+import org.springframework.cloud.client.circuitbreaker.ConfigBuilder;
+import org.springframework.cloud.client.circuitbreaker.NoFallbackAvailableException;
+import org.springframework.cloud.openfeign.EnableFeignClients;
+import org.springframework.cloud.openfeign.FeignClient;
+import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.util.SocketUtils;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RestController;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests for Feign calls with CircuitBreaker, without fallbacks.
+ *
+ * @author Olga Maciaszek-Sharma
+ */
+@SpringBootTest(classes = CircuitBreakerWithNoFallbackTests.Application.class,
+		webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT,
+		value = { "spring.application.name=springcircuittest", "spring.jmx.enabled=false",
+				"feign.circuitbreaker.enabled=true" })
+@DirtiesContext
+public class CircuitBreakerWithNoFallbackTests {
+
+	@Autowired
+	MyCircuitBreaker myCircuitBreaker;
+
+	@Autowired
+	CircuitBreakerTestClient testClient;
+
+	@BeforeAll
+	public static void beforeClass() {
+		System.setProperty("server.port",
+				String.valueOf(SocketUtils.findAvailableTcpPort()));
+	}
+
+	@AfterAll
+	public static void afterClass() {
+		System.clearProperty("server.port");
+	}
+
+	@BeforeEach
+	public void setup() {
+		this.myCircuitBreaker.clear();
+	}
+
+	@Test
+	public void testSimpleTypeWithFallback() {
+		Hello hello = testClient.getHello();
+
+		assertThat(hello).as("hello was null").isNotNull();
+		assertThat(hello).as("first hello didn't match")
+				.isEqualTo(new Hello("hello world 1"));
+		assertThat(myCircuitBreaker.runWasCalled).as("Circuit Breaker was called")
+				.isTrue();
+	}
+
+	@Test
+	public void test404WithoutFallback() {
+		assertThatThrownBy(() -> testClient.getException())
+				.isInstanceOf(NoFallbackAvailableException.class);
+	}
+
+	@FeignClient(name = "test", url = "http://localhost:${server.port}/")
+	protected interface CircuitBreakerTestClient {
+
+		@RequestMapping(method = RequestMethod.GET, value = "/hello")
+		Hello getHello();
+
+		@RequestMapping(method = RequestMethod.GET, value = "/hellonotfound")
+		String getException();
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@EnableAutoConfiguration
+	@RestController
+	@EnableFeignClients(clients = { CircuitBreakerTestClient.class })
+	@Import(NoSecurityConfiguration.class)
+	protected static class Application implements CircuitBreakerTestClient {
+
+		static final Log log = LogFactory.getLog(Application.class);
+
+		@Bean
+		MyCircuitBreaker myCircuitBreaker() {
+			return new MyCircuitBreaker();
+		}
+
+		@Bean
+		CircuitBreakerFactory circuitBreakerFactory(MyCircuitBreaker myCircuitBreaker) {
+			return new CircuitBreakerFactory() {
+				@Override
+				public CircuitBreaker create(String id) {
+					log.info("Creating a circuit breaker with id [" + id + "]");
+					return myCircuitBreaker;
+				}
+
+				@Override
+				protected ConfigBuilder configBuilder(String id) {
+					return Object::new;
+				}
+
+				@Override
+				public void configureDefault(Function defaultConfiguration) {
+
+				}
+			};
+		}
+
+		@Override
+		public Hello getHello() {
+			return new Hello("hello world 1");
+		}
+
+		@Override
+		public String getException() {
+			throw new IllegalStateException("BOOM!");
+		}
+
+	}
+
+}

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/Hello.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2013-2021 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.circuitbreaker;
+
+import java.util.Objects;
+
+/**
+ * @author Marcin Grzejszczak
+ */
+class Hello {
+
+	private String message;
+
+	public Hello() {
+	}
+
+	public Hello(String message) {
+		this.message = message;
+	}
+
+	public String getMessage() {
+		return this.message;
+	}
+
+	public void setMessage(String message) {
+		this.message = message;
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) {
+			return true;
+		}
+		if (o == null || getClass() != o.getClass()) {
+			return false;
+		}
+		Hello that = (Hello) o;
+		return Objects.equals(this.message, that.message);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(this.message);
+	}
+
+}

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/MyCircuitBreaker.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2013-2021 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.circuitbreaker;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
+import java.util.function.Supplier;
+
+import org.springframework.cloud.client.circuitbreaker.CircuitBreaker;
+import org.springframework.cloud.client.circuitbreaker.NoFallbackAvailableException;
+
+/**
+ * @author Marcin Grzejszczak
+ * @author Olga Maciaszek-Sharma
+ */
+class MyCircuitBreaker implements CircuitBreaker {
+
+	AtomicBoolean runWasCalled = new AtomicBoolean();
+
+	@Override
+	public <T> T run(Supplier<T> toRun) {
+		try {
+			this.runWasCalled.set(true);
+			return toRun.get();
+		}
+		catch (Throwable throwable) {
+			throw new NoFallbackAvailableException("No fallback available.", throwable);
+		}
+	}
+
+	@Override
+	public <T> T run(Supplier<T> toRun, Function<Throwable, T> fallback) {
+		try {
+			return run(toRun);
+		}
+		catch (Throwable throwable) {
+			return fallback.apply(throwable);
+		}
+	}
+
+	public void clear() {
+		this.runWasCalled.set(false);
+	}
+
+}

src/checkstyle/checkstyle-suppressions.xml

@@ -3,16 +3,19 @@
 		"-//Puppy Crawl//DTD Suppressions 1.1//EN"
 		"https://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
 <suppressions>
-	<suppress files=".*FeignClientTests.*" checks="LineLengthCheck" />
-	<suppress files=".*FeignClientsRegistrarTests.*" checks="LineLengthCheck" />
-	<suppress files=".*FeignRibbonHttpClientConfigurationTests.*" checks="LineLengthCheck" />
-	<suppress files=".*FeignRibbonOkHttpClientConfigurationTests.*" checks="LineLengthCheck" />
-	<suppress files=".*InvoiceClient.*" checks="LineLengthCheck" />
-	<suppress files=".*InvoiceResource.*" checks="LineLengthCheck" />
-	<suppress files=".*IterableParameterTests.*" checks="LineLengthCheck" />
-	<suppress files=".*LoadBalancerFeignClientOverrideTests.*" checks="LineLengthCheck" />
-	<suppress files=".*ProtobufSpringEncoderTest.*" checks="LineLengthCheck" />
-	<suppress files=".*ProtobufTest.*" checks="LineLengthCheck" />
+	<suppress files=".*FeignClientTests.*" checks="LineLengthCheck"/>
+	<suppress files=".*FeignClientsRegistrarTests.*" checks="LineLengthCheck"/>
+	<suppress files=".*FeignRibbonHttpClientConfigurationTests.*"
+			  checks="LineLengthCheck"/>
+	<suppress files=".*FeignRibbonOkHttpClientConfigurationTests.*"
+			  checks="LineLengthCheck"/>
+	<suppress files=".*InvoiceClient.*" checks="LineLengthCheck"/>
+	<suppress files=".*InvoiceResource.*" checks="LineLengthCheck"/>
+	<suppress files=".*IterableParameterTests.*" checks="LineLengthCheck"/>
+	<suppress files=".*LoadBalancerFeignClientOverrideTests.*" checks="LineLengthCheck"/>
+	<suppress files=".*ProtobufSpringEncoderTest.*" checks="LineLengthCheck"/>
+	<suppress files=".*ProtobufTest.*" checks="LineLengthCheck"/>
+	<suppress files=".*Hello.*" checks="RedundantModifier"/>
 	<!-- Important -->
-	<suppress files=".*HttpEncoding.*" checks="InterfaceIsTypeCheck" />
+	<suppress files=".*HttpEncoding.*" checks="InterfaceIsTypeCheck"/>
 </suppressions>