1.Filtering out default durations in the CompositeDelayFunction

2. Returning default duration from the CompositeDelayFunction instead of null
zalando · Nov 20, 2024 · 055fa29 · 055fa29
1 parent e4caf83
commit 055fa29
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 8 deletions.
diff --git a/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/CompositeDelayFunction.java b/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/CompositeDelayFunction.java
@@ -29,8 +29,9 @@ public Duration get(final ExecutionContext<R> context) throws Throwable {
                     }
                 })
                 .filter(Objects::nonNull)
+                .filter(delay -> !Duration.ofMinutes(-1).equals(delay))
                 .findFirst()
-                .orElse(null);
+                .orElse(Duration.ofMinutes(-1));
     }
 
     @SafeVarargs

diff --git a/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/CompositeDelayFunctionTest.java b/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/CompositeDelayFunctionTest.java
@@ -11,7 +11,6 @@
 import java.time.Duration;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -43,8 +42,8 @@ final class CompositeDelayFunctionTest {
     @BeforeEach
     void defaultBehavior() throws Throwable {
         // starting with Mockito 3.4.4, mocks will return Duration.ZERO instead of null, by default
-        when(first.get(any())).thenReturn(null);
-        when(first.get(any())).thenReturn(null);
+        when(first.get(any())).thenReturn(Duration.ofMinutes(-1));
+        when(first.get(any())).thenReturn(Duration.ofMinutes(-1));
     }
 
     @Test
@@ -73,11 +72,18 @@ void shouldIgnoreNullDelay() throws Throwable {
     }
 
     @Test
-    void shouldFallbackToNullDelay() throws Throwable {
+    void shouldFallbackToDefaultDelay() throws Throwable {
         when(first.get(eq(firstContext))).thenReturn(Duration.ofSeconds(1));
         when(second.get(eq(secondContext))).thenReturn(Duration.ofSeconds(2));
 
-        assertNull(unit.get(mock(ExecutionContext.class)));
+        assertEquals(Duration.ofMinutes(-1), unit.get(mock(ExecutionContext.class)));
+    }
+
+    @Test
+    void shouldUseFirstNonDefaultDelay() throws Throwable {
+        when(second.get(eq(secondContext))).thenReturn(Duration.ofSeconds(2));
+
+        assertEquals(Duration.ofSeconds(2), unit.get(secondContext));
     }
 
 }
diff --git a/...utoconfigure/src/test/java/org/zalando/riptide/autoconfigure/retry/FailsafeRetryTest.java b/...utoconfigure/src/test/java/org/zalando/riptide/autoconfigure/retry/FailsafeRetryTest.java
@@ -1,11 +1,13 @@
 package org.zalando.riptide.autoconfigure.retry;
 
+import com.google.common.base.Stopwatch;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpStatus;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.web.client.MockRestServiceServer;
 import org.zalando.logbook.autoconfigure.LogbookAutoConfiguration;
@@ -15,17 +17,27 @@
 import org.zalando.riptide.autoconfigure.RiptideClientTest;
 import org.zalando.riptide.failsafe.RetryException;
 
+import java.time.Duration;
 import java.util.concurrent.CompletionException;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.springframework.http.HttpStatus.Series.SERVER_ERROR;
+import static org.junit.jupiter.api.Assertions.assertTimeout;
 import static org.springframework.http.HttpStatus.Series.CLIENT_ERROR;
+import static org.springframework.http.HttpStatus.Series.SERVER_ERROR;
+import static org.springframework.http.HttpStatus.Series.SUCCESSFUL;
 import static org.springframework.test.web.client.ExpectedCount.times;
 import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
-import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError;
 import static org.springframework.test.web.client.response.MockRestResponseCreators.withBadRequest;
+import static org.springframework.test.web.client.response.MockRestResponseCreators.withServerError;
+import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
+import static org.springframework.test.web.client.response.MockRestResponseCreators.withTooManyRequests;
+import static org.zalando.riptide.Bindings.anySeries;
 import static org.zalando.riptide.Bindings.on;
 import static org.zalando.riptide.Navigators.series;
+import static org.zalando.riptide.Navigators.status;
+import static org.zalando.riptide.PassRoute.pass;
 import static org.zalando.riptide.failsafe.RetryRoute.retry;
 
 @RiptideClientTest
@@ -70,4 +82,34 @@ void shouldRetryForCustomRetryException() {
 
         server.verify();
     }
+
+    @Test
+    void shouldRetryWithDelayEpochSeconds() {
+        server.expect(times(1), requestTo("http://retry-test"))
+                .andRespond(withTooManyRequests().header("X-RateLimit-Reset", "2"));
+        server.expect(times(1), requestTo("http://retry-test")).andRespond(withSuccess());
+
+        assertTimeout(Duration.ofMillis(3000), () -> {
+            atLeast(Duration.ofSeconds(2), () -> retryClient.get()
+                    .dispatch(series(),
+                            on(SUCCESSFUL).call(pass()),
+                            anySeries().dispatch(status(),
+                                    on(HttpStatus.TOO_MANY_REQUESTS).call(retry())))
+                    .join());
+        });
+
+        server.verify();
+    }
+
+    private void atLeast(final Duration minimum, final Runnable runnable) {
+        final Duration actual = time(runnable);
+
+        assertThat(actual, greaterThanOrEqualTo(minimum));
+    }
+
+    private Duration time(final Runnable runnable) {
+        final Stopwatch stopwatch = Stopwatch.createStarted();
+        runnable.run();
+        return stopwatch.stop().elapsed();
+    }
 }