From ec45dd6dcab99ad6afc4b8efcaae2aca6073ed59 Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Wed, 4 Mar 2026 11:23:12 -0500 Subject: [PATCH] MLE-27388 Added test for 50x retry --- .../marklogic/client/impl/OkHttpServices.java | 18 +- .../marklogic/client/impl/RetryContext.java | 9 +- .../impl/okhttp/RetryOn50XResponseTest.java | 160 ++++++++++++++++++ .../src/test/resources/logback-test.xml | 4 + 4 files changed, 178 insertions(+), 13 deletions(-) create mode 100644 marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/RetryOn50XResponseTest.java diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java index bc2906909..573f89b0b 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java @@ -523,7 +523,7 @@ private Response sendRequestWithRetry( throw new MarkLogicIOException("Request cancelled: thread was interrupted before request could be sent"); } - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; @@ -1195,7 +1195,7 @@ private TemporalDescriptor putPostDocumentImpl(RequestLogger reqlog, String meth } boolean isResendable = handleBase.isResendable(); - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; Headers responseHeaders = null; @@ -1348,7 +1348,7 @@ private TemporalDescriptor putPostDocumentImpl(RequestLogger reqlog, String meth requestBldr = addVersionHeader(desc, requestBldr, "If-Match"); } - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; Headers responseHeaders = null; @@ -2107,7 +2107,7 @@ void init() { } Response getResponse() { - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, OkHttpServices.this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, OkHttpServices.this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { @@ -2635,7 +2635,7 @@ private void putPostValueImpl(RequestLogger reqlog, String method, String connectPath = null; Request.Builder requestBldr = null; - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { @@ -3060,7 +3060,7 @@ public R putResour String outputMimetype = outputBase.getMimetype(); Class as = outputBase.receiveAs(); - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { @@ -3223,7 +3223,7 @@ public R postResou String outputMimetype = outputBase != null ? outputBase.getMimetype() : null; Class as = outputBase != null ? outputBase.receiveAs() : null; - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { @@ -3822,7 +3822,7 @@ private U postIt throws ResourceNotFoundException, ResourceNotResendableException, ForbiddenUserException, FailedRequestException { if (params == null) params = new RequestParameters(); if (transaction != null) params.add("txid", transaction.getTransactionId()); - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { @@ -4928,7 +4928,7 @@ public InputStream match(QueryDefinition queryDef, } requestBldr = addTelemetryAgentId(requestBldr); - RetryContext retryContext = new RetryContext(logger, RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); + RetryContext retryContext = new RetryContext(RETRYABLE_STATUS_CODES, this::resetFirstRequestFlag); Response response = null; int status = -1; for (; retryContext.shouldContinueRetrying(minRetryAttempts, maxDelayForRetries); retryContext.incrementRetry()) { diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/RetryContext.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/RetryContext.java index fe17af67d..67185804a 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/RetryContext.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/RetryContext.java @@ -5,6 +5,7 @@ import com.marklogic.client.FailedRetryException; import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Set; @@ -13,7 +14,9 @@ * Tracks retry state, calculates delays, handles sleeping, and logs retry attempts. */ class RetryContext { - private final Logger logger; + + static final private Logger logger = LoggerFactory.getLogger(RetryContext.class); + private final Set retryableStatusCodes; private final Runnable onMaxRetriesCallback; @@ -22,12 +25,10 @@ class RetryContext { private int nextDelay = 0; /** - * @param logger Logger for debug output * @param retryableStatusCodes Set of HTTP status codes that trigger retries * @param onMaxRetriesCallback Callback to invoke when max retries is exceeded (e.g., to reset first request flag) */ - RetryContext(Logger logger, Set retryableStatusCodes, Runnable onMaxRetriesCallback) { - this.logger = logger; + RetryContext(Set retryableStatusCodes, Runnable onMaxRetriesCallback) { this.retryableStatusCodes = retryableStatusCodes; this.onMaxRetriesCallback = onMaxRetriesCallback; } diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/RetryOn50XResponseTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/RetryOn50XResponseTest.java new file mode 100644 index 000000000..db943ff4f --- /dev/null +++ b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/RetryOn50XResponseTest.java @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + */ +package com.marklogic.client.impl.okhttp; + +import com.marklogic.client.DatabaseClient; +import com.marklogic.client.DatabaseClientFactory; +import com.marklogic.client.FailedRequestException; +import com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator; +import com.marklogic.client.impl.RESTServices; +import com.marklogic.client.test.Common; +import okhttp3.Interceptor; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.Response; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests the retry logic in OkHttpServices using a custom interceptor that simulates server errors. This is not a + * well-documented feature but users are currently able to discover it via the codebase. + */ +class RetryOn50XResponseTest { + + /** + * Custom interceptor that returns 502 Bad Gateway responses and counts how many times it's invoked. + */ + private static class BadGatewayInterceptor implements Interceptor { + + private final AtomicInteger invocationCount = new AtomicInteger(0); + private final int failureCount; + + /** + * @param failureCount Number of times to return 502 before allowing the request through + */ + public BadGatewayInterceptor(int failureCount) { + this.failureCount = failureCount; + } + + @Override + public Response intercept(Chain chain) throws IOException { + int count = invocationCount.incrementAndGet(); + Request request = chain.request(); + + // Fail the first N requests + if (count <= failureCount) { + return new Response.Builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .code(502) + .message("Bad Gateway") + .body(okhttp3.ResponseBody.create("Simulated 502 error", null)) + .build(); + } + + // After N failures, let the request through + return chain.proceed(request); + } + + public int getInvocationCount() { + return invocationCount.get(); + } + + public void reset() { + invocationCount.set(0); + } + } + + @BeforeEach + void setUp() { + // Configure very short retry delays for testing + System.setProperty(RESTServices.MAX_DELAY_PROP, "3"); + System.setProperty(RESTServices.MIN_RETRY_PROP, "2"); + + // "Touch" the Common class to trigger the static block that removes existing configurators on + // DatabaseClientFactory. + Common.newServerPayload(); + } + + @AfterEach + void tearDown() { + DatabaseClientFactory.removeConfigurators(); + System.clearProperty(RESTServices.MAX_DELAY_PROP); + System.clearProperty(RESTServices.MIN_RETRY_PROP); + } + + @Test + void testRetryWith502Responses() { + // Create an interceptor that will fail 2 times, then succeed + BadGatewayInterceptor interceptor = new BadGatewayInterceptor(2); + + DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) builder -> + builder.addInterceptor(interceptor)); + + try (DatabaseClient client = Common.newClient()) { + client.checkConnection(); + assertEquals(3, interceptor.getInvocationCount(), + "Expected 3 invocations: 2 failures followed by 1 success"); + } + } + + @Test + void testRetryExceedsMaxAttempts() { + // Create an interceptor that will fail 10 times (more than minRetry) + BadGatewayInterceptor interceptor = new BadGatewayInterceptor(10); + + DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) builder -> + builder.addInterceptor(interceptor)); + + try (DatabaseClient client = Common.newClient()) { + assertThrows(FailedRequestException.class, () -> { + client.checkConnection(); + }, "Expected FailedRequestException after exhausting retries"); + + assertTrue(interceptor.getInvocationCount() >= 3, + "Expected at least 3 retry attempts, but got " + interceptor.getInvocationCount()); + } + } + + @Test + void testRetryCountIncreases() { + // Test that retry attempts increase as expected + BadGatewayInterceptor interceptor = new BadGatewayInterceptor(1); + + DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) builder -> + builder.addInterceptor(interceptor)); + + try (DatabaseClient client = Common.newClient()) { + // First request: fails once, then succeeds + client.checkConnection(); + assertEquals(2, interceptor.getInvocationCount(), "Expected 2 invocations for first request"); + + // Reset and try again + interceptor.reset(); + client.checkConnection(); + assertEquals(2, interceptor.getInvocationCount(), "Expected 2 invocations for second request"); + } + } + + @Test + void testNoRetryOnSuccessfulRequest() { + // Interceptor that never fails + BadGatewayInterceptor interceptor = new BadGatewayInterceptor(0); + + DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) builder -> + builder.addInterceptor(interceptor)); + + try (DatabaseClient client = Common.newClient()) { + client.checkConnection(); + assertEquals(1, interceptor.getInvocationCount(), + "Expected exactly 1 invocation when request succeeds immediately"); + } + } +} diff --git a/marklogic-client-api/src/test/resources/logback-test.xml b/marklogic-client-api/src/test/resources/logback-test.xml index 890be1792..6a3506a32 100644 --- a/marklogic-client-api/src/test/resources/logback-test.xml +++ b/marklogic-client-api/src/test/resources/logback-test.xml @@ -16,4 +16,8 @@ + + + +