From 9bb2d22b398019cc25c0cf64089655d963363b4e Mon Sep 17 00:00:00 2001 From: Sawjan Gurung Date: Mon, 12 Oct 2026 12:37:35 +0545 Subject: [PATCH] test(api): simply HTTP request retry conditions (#3696) Signed-off-by: Saw-jan --- .../TestHelpers/HttpRequestHelper.php | 70 ++++++++----------- tests/acceptance/bootstrap/WebDav.php | 3 +- 2 files changed, 30 insertions(+), 43 deletions(-) diff --git a/tests/acceptance/TestHelpers/HttpRequestHelper.php b/tests/acceptance/TestHelpers/HttpRequestHelper.php index 41ecd4d9d6..3033393e93 100644 --- a/tests/acceptance/TestHelpers/HttpRequestHelper.php +++ b/tests/acceptance/TestHelpers/HttpRequestHelper.php @@ -39,9 +39,6 @@ use Symfony\Component\HttpFoundation\Response; * Helper for HTTP requests */ class HttpRequestHelper { - public const HTTP_TOO_EARLY = 425; - public const HTTP_CONFLICT = 409; - /** * Some systems-under-test do async post-processing of operations like upload, * move, etc. If a client does a request on the resource before the post-processing @@ -256,9 +253,7 @@ class HttpRequestHelper { $debugResponses = false; } - $sendRetryLimit = self::numRetriesOnHttpTooEarly(); $sendCount = 0; - $sendExceptionHappened = false; do { $response = self::sendRequestOnce( $url, @@ -275,42 +270,36 @@ class HttpRequestHelper { $client, ); - $isMigrating = $response->getStatusCode() === 500 - && self::responseIndicatesShareManagerMigrating($response); - - if ($response->getStatusCode() >= 400 - && $response->getStatusCode() !== self::HTTP_TOO_EARLY - && $response->getStatusCode() !== self::HTTP_CONFLICT - && !$isMigrating - ) { - $sendExceptionHappened = true; - } - if ($debugResponses) { self::debugResponse($response); } - $sendCount = $sendCount + 1; - // Here we check if the response has status code 425 or is a 409 gotten from a Given step - // HTTP_TOO_EARLY (425) can happen if async processing of a previous request is still happening. - // For example, if a test uploads a file and then immediately tries to download it. - // HTTP_CONFLICT (409) can happen if the user has just been created in the previous step. - // The OCS API might not "realize" yet that the user exists. A folder creation (MKCOL) or maybe even - // a file upload might return 409. - // share-manager-migrating (500) can happen right after oc_wrapper restarts server - // while the jsoncs3 share manager is still running its data migrations. - $loopAgain = !$sendExceptionHappened && ($response->getStatusCode() === self::HTTP_TOO_EARLY || - ($response->getStatusCode() === self::HTTP_CONFLICT && $isGivenStep) || - $isMigrating) && - $sendCount <= $sendRetryLimit; - if ($loopAgain) { - // we need to repeat the send request, because we got HTTP_TOO_EARLY or HTTP_CONFLICT - // wait 1 second before sending again, to give the server some time - // to finish whatever post-processing it might be doing. - echo "[INFO] Received '" . $response->getStatusCode() . - "' status code, retrying request ($sendCount)...\n"; - \sleep(1); + + $statusCode = $response->getStatusCode(); + $tooEarly = $statusCode === Response::HTTP_TOO_EARLY; + $conflict = $statusCode === Response::HTTP_CONFLICT; + $retryOnConflict = $conflict && $isGivenStep; + $isShareMigrating = $statusCode === Response::HTTP_INTERNAL_SERVER_ERROR + && self::responseIndicatesShareManagerMigrating($response); + + // Retry request only if: + // 1. we get HTTP_TOO_EARLY (425) - can happen if async processing is still happening. + // For example, if a test uploads a file and then immediately tries to download it. + // 2. we get HTTP_CONFLICT (409) and is Given step - can happen if the user has just been created. + // The OCS API might not "realize" yet that the user exists. A folder creation (MKCOL) or maybe even + // a file upload might return 409. + // 3. share manager is migrating (500) - can happen right after oc_wrapper restarts server + // while the jsoncs3 share manager is still running its data migrations. + if (!($tooEarly || $retryOnConflict || $isShareMigrating)) { + break; } - } while ($loopAgain); + + $sendCount = $sendCount + 1; + echo "[INFO] Received '" . $statusCode . + "' status code, retrying request ($sendCount)...\n"; + // wait 1 second before sending again, to give the server some time + // to finish whatever post-processing it might be doing. + \sleep(1); + } while ($sendCount <= self::numRetriesOnHttpTooEarly()); return $response; } @@ -354,11 +343,8 @@ class HttpRequestHelper { * @return bool */ private static function responseIndicatesShareManagerMigrating(ResponseInterface $response): bool { - $stream = $response->getBody(); - $body = $stream->getContents(); - if ($stream->isSeekable()) { - $stream->rewind(); - } + $body = $response->getBody()->getContents(); + $response->getBody()->rewind(); return \str_contains($body, 'share manager is currently migrating'); } diff --git a/tests/acceptance/bootstrap/WebDav.php b/tests/acceptance/bootstrap/WebDav.php index 45e9442735..d3e043f6bf 100644 --- a/tests/acceptance/bootstrap/WebDav.php +++ b/tests/acceptance/bootstrap/WebDav.php @@ -35,6 +35,7 @@ use TestHelpers\GraphHelper; use Behat\Step\Given; use Behat\Step\Then; use Behat\Step\When; +use Symfony\Component\HttpFoundation\Response; /** * WebDav functions @@ -976,7 +977,7 @@ trait WebDav { echo "[INFO] File content mismatch. $message, checking content again ($retried)...\n"; // break the loop if status is 425 as the request will already be retried - if ($status === HttpRequestHelper::HTTP_TOO_EARLY) { + if ($status === Response::HTTP_TOO_EARLY) { break; }