Merge pull request #428 from zalando/bugfix/default-circuit-breaker

Removed default circuit breaker
zalando · Jul 11, 2018 · 27f419a · 27f419a
2 parents af719cd + 0e4e12f
commit 27f419a
Show file tree

Hide file tree

Showing 5 changed files with 199 additions and 14 deletions.
diff --git a/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/FailsafePlugin.java b/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/FailsafePlugin.java
@@ -2,18 +2,20 @@
 
 import net.jodah.failsafe.CircuitBreaker;
 import net.jodah.failsafe.ExecutionContext;
-import net.jodah.failsafe.Failsafe;
 import net.jodah.failsafe.Listeners;
 import net.jodah.failsafe.RetryPolicy;
+import net.jodah.failsafe.SyncFailsafe;
 import org.apiguardian.api.API;
 import org.springframework.http.client.ClientHttpResponse;
 import org.zalando.riptide.Plugin;
 import org.zalando.riptide.RequestArguments;
 import org.zalando.riptide.RequestExecution;
 
+import javax.annotation.Nullable;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ScheduledExecutorService;
 
+import static net.jodah.failsafe.Failsafe.with;
 import static org.apiguardian.api.API.Status.STABLE;
 import static org.zalando.riptide.CancelableCompletableFuture.forwardTo;
 import static org.zalando.riptide.CancelableCompletableFuture.preserveCancelability;
@@ -29,11 +31,11 @@ public final class FailsafePlugin implements Plugin {
     private final RetryListener listener;
 
     public FailsafePlugin(final ScheduledExecutorService scheduler) {
-        this(scheduler, NEVER, new CircuitBreaker(), RetryListener.DEFAULT);
+        this(scheduler, NEVER, null, RetryListener.DEFAULT);
     }
 
     private FailsafePlugin(final ScheduledExecutorService scheduler, final RetryPolicy retryPolicy,
-            final CircuitBreaker circuitBreaker, final RetryListener listener) {
+            @Nullable final CircuitBreaker circuitBreaker, final RetryListener listener) {
         this.scheduler = scheduler;
         this.retryPolicy = retryPolicy;
         this.circuitBreaker = circuitBreaker;
@@ -55,9 +57,8 @@ public FailsafePlugin withListener(final RetryListener listener) {
     @Override
     public RequestExecution prepare(final RequestArguments arguments, final RequestExecution execution) {
         return () -> {
-            final CompletableFuture<ClientHttpResponse> original = Failsafe
-                    .with(retryPolicy)
-                    .with(circuitBreaker)
+            final CompletableFuture<ClientHttpResponse> original =
+                    failsafe()
                     .with(scheduler)
                     .with(new RetryListenersAdapter(listener, arguments))
                     .future(execution::execute);
@@ -68,6 +69,11 @@ public RequestExecution prepare(final RequestArguments arguments, final RequestE
         };
     }
 
+    SyncFailsafe<Object> failsafe() {
+        final SyncFailsafe<Object> failsafe = with(retryPolicy);
+        return circuitBreaker == null ? failsafe : failsafe.with(circuitBreaker);
+    }
+
     private static final class RetryListenersAdapter extends Listeners<ClientHttpResponse> {
 
         private final RequestArguments arguments;

diff --git a/...failsafe/src/test/java/org/zalando/riptide/failsafe/FailsafePluginCircuitBreakerTest.java b/...failsafe/src/test/java/org/zalando/riptide/failsafe/FailsafePluginCircuitBreakerTest.java
@@ -0,0 +1,94 @@
+package org.zalando.riptide.failsafe;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.restdriver.clientdriver.ClientDriverRule;
+import net.jodah.failsafe.CircuitBreaker;
+import net.jodah.failsafe.CircuitBreakerOpenException;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
+import org.springframework.scheduling.concurrent.ConcurrentTaskExecutor;
+import org.zalando.riptide.Http;
+import org.zalando.riptide.OriginalStackTracePlugin;
+import org.zalando.riptide.httpclient.RestAsyncClientHttpRequestFactory;
+
+import java.io.IOException;
+import java.net.SocketTimeoutException;
+import java.util.concurrent.CompletionException;
+
+import static com.github.restdriver.clientdriver.RestClientDriver.giveEmptyResponse;
+import static com.github.restdriver.clientdriver.RestClientDriver.onRequestTo;
+import static java.util.concurrent.Executors.newSingleThreadExecutor;
+import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.mockito.Mockito.mock;
+import static org.zalando.fauxpas.FauxPas.partially;
+import static org.zalando.riptide.PassRoute.pass;
+
+public class FailsafePluginCircuitBreakerTest {
+
+    @Rule
+    public final ClientDriverRule driver = new ClientDriverRule();
+
+    private final CloseableHttpClient client = HttpClientBuilder.create()
+            .setDefaultRequestConfig(RequestConfig.custom()
+                    .setSocketTimeout(500)
+                    .build())
+            .build();
+
+    private final RetryListener listeners = mock(RetryListener.class);
+
+    private final Http unit = Http.builder()
+            .baseUrl(driver.getBaseUrl())
+            .requestFactory(new RestAsyncClientHttpRequestFactory(client,
+                    new ConcurrentTaskExecutor(newSingleThreadExecutor())))
+            .converter(createJsonConverter())
+            .plugin(new FailsafePlugin(newSingleThreadScheduledExecutor())
+                    .withCircuitBreaker(new CircuitBreaker()
+                            .withDelay(1, SECONDS))
+                    .withListener(listeners))
+            .plugin(new OriginalStackTracePlugin())
+            .build();
+
+    private static MappingJackson2HttpMessageConverter createJsonConverter() {
+        final MappingJackson2HttpMessageConverter converter = new MappingJackson2HttpMessageConverter();
+        converter.setObjectMapper(createObjectMapper());
+        return converter;
+    }
+
+    private static ObjectMapper createObjectMapper() {
+        return new ObjectMapper().findAndRegisterModules()
+                .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+    }
+
+    @After
+    public void tearDown() throws IOException {
+        client.close();
+    }
+
+    @Test(expected = CircuitBreakerOpenException.class)
+    public void shouldOpenCircuit() throws Throwable {
+        driver.addExpectation(onRequestTo("/foo"), giveEmptyResponse().after(800, MILLISECONDS));
+
+        unit.get("/foo").call(pass())
+                .exceptionally(partially(SocketTimeoutException.class, this::ignore))
+                .join();
+
+        try {
+            unit.get("/foo").call(pass()).join();
+        } catch (final CompletionException e) {
+            throw e.getCause();
+        }
+    }
+
+    private Void ignore(@SuppressWarnings("unused") final Throwable throwable) {
+        return null;
+    }
+
+}
diff --git a/...ilsafe/src/test/java/org/zalando/riptide/failsafe/FailsafePluginNoCircuitBreakerTest.java b/...ilsafe/src/test/java/org/zalando/riptide/failsafe/FailsafePluginNoCircuitBreakerTest.java
@@ -0,0 +1,91 @@
+package org.zalando.riptide.failsafe;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.restdriver.clientdriver.ClientDriverRule;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
+import org.springframework.scheduling.concurrent.ConcurrentTaskExecutor;
+import org.zalando.riptide.Http;
+import org.zalando.riptide.OriginalStackTracePlugin;
+import org.zalando.riptide.httpclient.RestAsyncClientHttpRequestFactory;
+
+import java.io.IOException;
+import java.net.SocketTimeoutException;
+import java.util.concurrent.CompletableFuture;
+
+import static com.github.restdriver.clientdriver.RestClientDriver.giveEmptyResponse;
+import static com.github.restdriver.clientdriver.RestClientDriver.onRequestTo;
+import static java.util.concurrent.Executors.newSingleThreadExecutor;
+import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.mockito.Mockito.mock;
+import static org.zalando.fauxpas.FauxPas.partially;
+import static org.zalando.riptide.PassRoute.pass;
+
+public class FailsafePluginNoCircuitBreakerTest {
+
+    @Rule
+    public final ClientDriverRule driver = new ClientDriverRule();
+
+    private final CloseableHttpClient client = HttpClientBuilder.create()
+            .setDefaultRequestConfig(RequestConfig.custom()
+                    .setSocketTimeout(500)
+                    .build())
+            .build();
+
+    private final RetryListener listeners = mock(RetryListener.class);
+
+    private final Http unit = Http.builder()
+            .baseUrl(driver.getBaseUrl())
+            .requestFactory(new RestAsyncClientHttpRequestFactory(client,
+                    new ConcurrentTaskExecutor(newSingleThreadExecutor())))
+            .converter(createJsonConverter())
+            .plugin(new FailsafePlugin(newSingleThreadScheduledExecutor())
+                    .withListener(listeners))
+            .plugin(new OriginalStackTracePlugin())
+            .build();
+
+    private static MappingJackson2HttpMessageConverter createJsonConverter() {
+        final MappingJackson2HttpMessageConverter converter = new MappingJackson2HttpMessageConverter();
+        converter.setObjectMapper(createObjectMapper());
+        return converter;
+    }
+
+    private static ObjectMapper createObjectMapper() {
+        return new ObjectMapper().findAndRegisterModules()
+                .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+    }
+
+    @After
+    public void tearDown() throws IOException {
+        client.close();
+    }
+
+    @Test
+    public void shouldNotOpenCircuit() {
+        driver.addExpectation(onRequestTo("/foo"), giveEmptyResponse().after(800, MILLISECONDS));
+        driver.addExpectation(onRequestTo("/foo"), giveEmptyResponse().after(800, MILLISECONDS));
+        driver.addExpectation(onRequestTo("/foo"), giveEmptyResponse());
+
+        unit.get("/foo").call(pass())
+                .exceptionally(this::ignore)
+                .join();
+
+        final CompletableFuture<Void> timeout = unit.get("/foo").call(pass());
+        final CompletableFuture<Void> last = unit.get("/foo").call(pass());
+
+        timeout.exceptionally(partially(SocketTimeoutException.class, this::ignore)).join();
+        last.join();
+    }
+
+    private Void ignore(@SuppressWarnings("unused") final Throwable throwable) {
+        return null;
+    }
+
+}
diff --git a/.../riptide/failsafe/FailsafePluginTest.java → ...e/failsafe/FailsafePluginRetriesTest.java b/.../riptide/failsafe/FailsafePluginTest.java → ...e/failsafe/FailsafePluginRetriesTest.java
@@ -45,7 +45,7 @@
 import static org.zalando.riptide.PassRoute.pass;
 import static org.zalando.riptide.failsafe.RetryRoute.retry;
 
-public class FailsafePluginTest {
+public class FailsafePluginRetriesTest {
 
     @Rule
     public final ClientDriverRule driver = new ClientDriverRule();

diff --git a/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/RetryAfterDelayFunctionTest.java b/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/RetryAfterDelayFunctionTest.java
@@ -4,7 +4,6 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.restdriver.clientdriver.ClientDriverRule;
 import com.google.common.base.Stopwatch;
-import net.jodah.failsafe.CircuitBreaker;
 import net.jodah.failsafe.RetryPolicy;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.impl.client.CloseableHttpClient;
@@ -21,7 +20,6 @@
 import java.io.IOException;
 import java.time.Clock;
 import java.time.Duration;
-import java.util.concurrent.TimeUnit;
 
 import static com.github.restdriver.clientdriver.RestClientDriver.giveEmptyResponse;
 import static com.github.restdriver.clientdriver.RestClientDriver.onRequestTo;
@@ -62,11 +60,7 @@ public class RetryAfterDelayFunctionTest {
                     .withRetryPolicy(new RetryPolicy()
                             .withDelay(2, SECONDS)
                             .withDelay(new RetryAfterDelayFunction(clock))
-                            .withMaxRetries(4))
-                    .withCircuitBreaker(new CircuitBreaker()
-                            .withFailureThreshold(3, 10)
-                            .withSuccessThreshold(5)
-                            .withDelay(1, TimeUnit.MINUTES)))
+                            .withMaxRetries(4)))
             .build();
 
     private static MappingJackson2HttpMessageConverter createJsonConverter() {