feat(http): add option to retry only unprocessed mutations

components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RetryHandler.java

@@ -105,6 +105,7 @@ boolean retryRequest(
                         && executionCount <= retryOption.maxRetries()
                         && checkStatus(statusCode)
                         && isBuffered(request)
+                        && isRetriableForMethod(request, statusCode, retryOption)
                         && shouldRetryCallback.shouldRetry(
                                 retryOption.delay(), executionCount, request, response);
 
@@ -202,6 +203,33 @@ boolean checkStatus(int statusCode) {
                 || statusCode == MSClientErrorCodeGatewayTimeout);
     }
 
+    /**
+     * Checks whether the response allows resending the request, given its method.
+     * With retryUnprocessedMutationsOnly set, POST and PATCH requests are retried only after a
+     * response showing the server didn't process them, because a 504 may arrive after the server
+     * already acted on the request.
+     * @param request the request that would be resent
+     * @param statusCode the status code of the response
+     * @param retryOption the retry options in use
+     * @return whether the request can be resent
+     */
+    boolean isRetriableForMethod(
+            @Nonnull final Request request,
+            int statusCode,
+            @Nonnull final RetryHandlerOption retryOption) {
+        if (!retryOption.retryUnprocessedMutationsOnly()) {
+            return true;
+        }
+        final String methodName = request.method();
+        if (!methodName.equalsIgnoreCase("POST") && !methodName.equalsIgnoreCase("PATCH")) {
+            return true;
+        }
+        // By common service convention, a 429 or 503 means the server didn't process the request,
+        // so resending it is treated as safe.
+        return statusCode == MSClientErrorCodeTooManyRequests
+                || statusCode == MSClientErrorCodeServiceUnavailable;
+    }
+
     boolean isBuffered(final Request request) {
         final String methodName = request.method();
 

components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/RetryHandlerOption.java

@@ -44,6 +44,8 @@ public class RetryHandlerOption implements RequestOption {
      */
     public static final long MAX_DELAY = 180;
 
+    private final boolean mRetryUnprocessedMutationsOnly;
+
     /**
      * Create default instance of retry options, with default values of delay, max retries and shouldRetry callback.
      */
@@ -60,6 +62,22 @@ public RetryHandlerOption() {
     @SuppressWarnings("LambdaLast")
     public RetryHandlerOption(
             @Nullable final IShouldRetry shouldRetry, int maxRetries, long delay) {
+        this(shouldRetry, maxRetries, delay, false);
+    }
+
+    /**
+     * Create an instance with provided values
+     * @param shouldRetry Retry callback to be called before making a retry
+     * @param maxRetries Number of max retires for a request
+     * @param delay Delay in seconds between retries
+     * @param retryUnprocessedMutationsOnly Whether POST and PATCH requests are retried only after a 429 or 503 response, see {@link #retryUnprocessedMutationsOnly()}
+     */
+    @SuppressWarnings("LambdaLast")
+    public RetryHandlerOption(
+            @Nullable final IShouldRetry shouldRetry,
+            int maxRetries,
+            long delay,
+            boolean retryUnprocessedMutationsOnly) {
         if (delay > MAX_DELAY)
             throw new IllegalArgumentException("Delay cannot exceed " + MAX_DELAY);
         if (delay < 0) throw new IllegalArgumentException("Delay cannot be negative");
@@ -70,6 +88,7 @@ public RetryHandlerOption(
         this.mShouldRetry = shouldRetry == null ? DEFAULT_SHOULD_RETRY : shouldRetry;
         this.mMaxRetries = maxRetries;
         this.mDelay = delay;
+        this.mRetryUnprocessedMutationsOnly = retryUnprocessedMutationsOnly;
     }
 
     /**
@@ -96,6 +115,21 @@ public long delay() {
         return mDelay;
     }
 
+    /**
+     * Gets whether POST and PATCH requests are retried only after a response showing the server didn't process them. This defaults to false.
+     * <p>
+     * POST and PATCH aren't idempotent, so resending one the server already processed can repeat its side effect, such as sending an email twice.
+     * A 504 can arrive after the server processed the request.
+     * When this is true, POST and PATCH requests are retried only after a 429 or 503 response, and a 504 is returned to the caller.
+     * This relies on the common service convention, followed by Microsoft Graph, that a 429 or 503 response means the server didn't process the request.
+     * HTTP doesn't guarantee this, so a service that returns 429 or 503 after acting on a request can still see it repeated.
+     * PUT and DELETE also change state, but they're idempotent, so they aren't affected, and neither are other methods.
+     * @return whether POST and PATCH requests are retried only after a 429 or 503 response
+     */
+    public boolean retryUnprocessedMutationsOnly() {
+        return mRetryUnprocessedMutationsOnly;
+    }
+
     /** {@inheritDoc} */
     @Override
     @SuppressWarnings("unchecked")

components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RetryHandlerTest.java

@@ -1,5 +1,6 @@
 package com.microsoft.kiota.http.middleware;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -228,4 +229,173 @@ void interceptExecutesCleanlyWhenObservabilitySpanIsNull() throws Exception {
             assertEquals("1", retry.getHeader("Retry-Attempt"));
         }
     }
+
+    private static RetryHandlerOption retryUnprocessedMutationsOnlyOption() {
+        return new RetryHandlerOption((delay, executionCount, req, resp) -> true, 3, 0, true);
+    }
+
+    private static int sendWithStatuses(
+            final RetryHandlerOption handlerOption,
+            final RetryHandlerOption requestOption,
+            final String method,
+            final int... statusCodes)
+            throws Exception {
+        try (var server = new MockWebServer()) {
+            for (final int statusCode : statusCodes) {
+                server.enqueue(
+                        new MockResponse()
+                                .setResponseCode(statusCode)
+                                .setHeader("Retry-After", "0"));
+            }
+            server.enqueue(new MockResponse().setResponseCode(200));
+            final OkHttpClient client =
+                    new OkHttpClient.Builder()
+                            .addInterceptor(new RetryHandler(handlerOption))
+                            .build();
+            final RequestBody body =
+                    method.equalsIgnoreCase("GET") || method.equalsIgnoreCase("DELETE")
+                            ? null
+                            : RequestBody.create("{\"message\":\"hello\"}", null);
+            final Request.Builder builder =
+                    new Request.Builder().url(server.url("/send")).method(method, body);
+            if (requestOption != null) {
+                builder.tag(RetryHandlerOption.class, requestOption);
+            }
+            client.newCall(builder.build()).execute().close();
+            return server.getRequestCount();
+        }
+    }
+
+    @Test
+    void retryUnprocessedMutationsOnlyDefaultsToFalse() {
+        assertFalse(new RetryHandlerOption().retryUnprocessedMutationsOnly());
+        assertFalse(
+                new RetryHandlerOption((delay, executionCount, req, resp) -> true, 3, 0)
+                        .retryUnprocessedMutationsOnly());
+        assertTrue(retryUnprocessedMutationsOnlyOption().retryUnprocessedMutationsOnly());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"POST", "PATCH"})
+    void retriesNonIdempotentRequestOnGatewayTimeoutByDefault(String method) throws Exception {
+        final var option = new RetryHandlerOption((delay, executionCount, req, resp) -> true, 3, 0);
+        assertEquals(2, sendWithStatuses(option, null, method, 504));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"POST", "PATCH"})
+    void doesNotRetryNonIdempotentRequestOnGatewayTimeoutWhenRetryUnprocessedMutationsOnly(
+            String method) throws Exception {
+        assertEquals(1, sendWithStatuses(retryUnprocessedMutationsOnlyOption(), null, method, 504));
+    }
+
+    @Test
+    void matchesNonIdempotentMethodsIgnoringCase() {
+        final Request request =
+                new Request.Builder()
+                        .url("https://example.org/send")
+                        .method("post", RequestBody.create("{}", null))
+                        .build();
+        assertFalse(
+                retryHandler.isRetriableForMethod(
+                        request, 504, retryUnprocessedMutationsOnlyOption()));
+        assertTrue(
+                retryHandler.isRetriableForMethod(
+                        request, 503, retryUnprocessedMutationsOnlyOption()));
+    }
+
+    private static Stream<Arguments> nonIdempotentMethodsAndUnprocessedStatuses() {
+        return Stream.of(
+                Arguments.of("POST", 429),
+                Arguments.of("POST", 503),
+                Arguments.of("PATCH", 429),
+                Arguments.of("PATCH", 503));
+    }
+
+    @ParameterizedTest
+    @MethodSource("nonIdempotentMethodsAndUnprocessedStatuses")
+    void retriesNonIdempotentRequestWhenNotProcessedAndRetryUnprocessedMutationsOnly(
+            String method, int statusCode) throws Exception {
+        assertEquals(
+                2,
+                sendWithStatuses(retryUnprocessedMutationsOnlyOption(), null, method, statusCode));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"GET", "PUT", "DELETE", "QUERY"})
+    void retriesIdempotentRequestOnGatewayTimeoutWhenRetryUnprocessedMutationsOnly(String method)
+            throws Exception {
+        assertEquals(2, sendWithStatuses(retryUnprocessedMutationsOnlyOption(), null, method, 504));
+    }
+
+    @Test
+    void stopsRetryingNonIdempotentRequestWhenALaterResponseLeavesOutcomeUnknown()
+            throws Exception {
+        assertEquals(
+                2, sendWithStatuses(retryUnprocessedMutationsOnlyOption(), null, "POST", 503, 504));
+    }
+
+    @Test
+    void honoursRetryUnprocessedMutationsOnlyFromRequestOption() throws Exception {
+        final var handlerOption =
+                new RetryHandlerOption((delay, executionCount, req, resp) -> true, 3, 0);
+        assertEquals(
+                1,
+                sendWithStatuses(
+                        handlerOption, retryUnprocessedMutationsOnlyOption(), "POST", 504));
+    }
+
+    private static String[] sendThroughRedirect(
+            final RetryHandlerOption handlerOption,
+            final RetryHandlerOption requestOption,
+            final int redirectStatus,
+            final int expectedStatus)
+            throws Exception {
+        try (var server = new MockWebServer()) {
+            server.enqueue(
+                    new MockResponse()
+                            .setResponseCode(redirectStatus)
+                            .setHeader("Location", server.url("/redirected")));
+            server.enqueue(new MockResponse().setResponseCode(504).setHeader("Retry-After", "0"));
+            server.enqueue(new MockResponse().setResponseCode(200));
+            final OkHttpClient client =
+                    new OkHttpClient.Builder()
+                            .followRedirects(false)
+                            .addInterceptor(new RedirectHandler())
+                            .addInterceptor(new RetryHandler(handlerOption))
+                            .build();
+            final Request.Builder builder =
+                    new Request.Builder()
+                            .url(server.url("/send"))
+                            .post(RequestBody.create("{\"message\":\"hello\"}", null));
+            if (requestOption != null) {
+                builder.tag(RetryHandlerOption.class, requestOption);
+            }
+            try (var response = client.newCall(builder.build()).execute()) {
+                assertEquals(expectedStatus, response.code());
+            }
+            final String[] methods = new String[server.getRequestCount()];
+            for (int i = 0; i < methods.length; i++) {
+                methods[i] = server.takeRequest().getMethod();
+            }
+            return methods;
+        }
+    }
+
+    @Test
+    void retriesPostRedirectedToGetWhenRetryUnprocessedMutationsOnly() throws Exception {
+        assertArrayEquals(
+                new String[] {"POST", "GET", "GET"},
+                sendThroughRedirect(retryUnprocessedMutationsOnlyOption(), null, 302, 200));
+    }
+
+    @Test
+    void doesNotRetryPostRedirectedAsPostWithRequestOption() throws Exception {
+        final var handlerOption =
+                new RetryHandlerOption((delay, executionCount, req, resp) -> true, 3, 0);
+        assertArrayEquals(
+                new String[] {"POST", "POST"},
+                sendThroughRedirect(
+                        handlerOption, retryUnprocessedMutationsOnlyOption(), 307, 504));
+    }
 }