From bbef08205d53a3bf4a51f9649ae1c3ef9586f173 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 22 Jan 2021 16:53:32 +0100 Subject: [PATCH] Use CircuitBreaker even when no fallback. (#466) --- .../cloud/openfeign/FeignCircuitBreaker.java | 22 +-- ...kerTests.java => CircuitBreakerTests.java} | 73 +------- .../CircuitBreakerWithNoFallbackTests.java | 159 ++++++++++++++++++ .../cloud/openfeign/circuitbreaker/Hello.java | 60 +++++++ .../circuitbreaker/MyCircuitBreaker.java | 59 +++++++ src/checkstyle/checkstyle-suppressions.xml | 25 +-- 6 files changed, 304 insertions(+), 94 deletions(-) rename spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/{CirciutBreakerTests.java => CircuitBreakerTests.java} (83%) create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerWithNoFallbackTests.java create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/Hello.java create mode 100644 spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/MyCircuitBreaker.java diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreaker.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreaker.java index 8d306890..6b970f60 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreaker.java +++ b/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 target(Target target) { + return build(null).newInstance(target); + } + public Feign build(final FallbackFactory nullableFallbackFactory) { - super.invocationHandlerFactory(new InvocationHandlerFactory() { - @Override - public InvocationHandler create(Target target, - Map dispatch) { - return new FeignCircuitBreakerInvocationHandler(circuitBreakerFactory, - feignClientName, target, dispatch, nullableFallbackFactory); - } - }); + super.invocationHandlerFactory( + (target, dispatch) -> new FeignCircuitBreakerInvocationHandler( + circuitBreakerFactory, feignClientName, target, dispatch, + nullableFallbackFactory)); return super.build(); } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CirciutBreakerTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java similarity index 83% rename from spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CirciutBreakerTests.java rename to spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java index 0555aafe..84bb2531 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CirciutBreakerTests.java +++ b/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 run(Supplier toRun) { - this.runWasCalled.set(true); - return toRun.get(); - } - - @Override - public T run(Supplier toRun, Function fallback) { - try { - return run(toRun); - } - catch (Throwable throwable) { - return fallback.apply(throwable); - } - } - - public void clear() { - this.runWasCalled.set(false); - } - - } - } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerWithNoFallbackTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerWithNoFallbackTests.java new file mode 100644 index 00000000..9c42e00a --- /dev/null +++ b/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!"); + } + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/Hello.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/Hello.java new file mode 100644 index 00000000..a6e66e4a --- /dev/null +++ b/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); + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/MyCircuitBreaker.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/MyCircuitBreaker.java new file mode 100644 index 00000000..475cb6c0 --- /dev/null +++ b/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 run(Supplier toRun) { + try { + this.runWasCalled.set(true); + return toRun.get(); + } + catch (Throwable throwable) { + throw new NoFallbackAvailableException("No fallback available.", throwable); + } + } + + @Override + public T run(Supplier toRun, Function fallback) { + try { + return run(toRun); + } + catch (Throwable throwable) { + return fallback.apply(throwable); + } + } + + public void clear() { + this.runWasCalled.set(false); + } + +} diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index 0204a3ed..39915018 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/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"> - - - - - - - - - - + + + + + + + + + + + - +