From abb111c3a1311e67b08a0df9d0a3becf29a79b3d Mon Sep 17 00:00:00 2001 From: Niraj Acharya Date: Thu, 13 Jul 2023 12:44:51 +0545 Subject: [PATCH] if a requests response is 409 retry upto 10 times --- tests/TestHelpers/HttpRequestHelper.php | 24 ++++++++++++----- tests/TestHelpers/WebDavHelper.php | 7 +++-- .../acceptance/features/bootstrap/WebDav.php | 27 ++++++++++++++----- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/tests/TestHelpers/HttpRequestHelper.php b/tests/TestHelpers/HttpRequestHelper.php index 6ea218786..aab62600d 100644 --- a/tests/TestHelpers/HttpRequestHelper.php +++ b/tests/TestHelpers/HttpRequestHelper.php @@ -41,7 +41,7 @@ use GuzzleHttp\Pool; */ class HttpRequestHelper { public const HTTP_TOO_EARLY = 425; - + public const HTTP_CONFLICT = 409; private static ?string $oCSelectorCookie = null; /** @@ -159,7 +159,6 @@ class HttpRequestHelper { } /** - * * @param string|null $url * @param string|null $xRequestId * @param string|null $method @@ -173,8 +172,10 @@ class HttpRequestHelper { * than download it all up-front. * @param int|null $timeout * @param Client|null $client + * @param bool|null $isGivenStep * * @return ResponseInterface + * * @throws GuzzleException */ public static function sendRequest( @@ -189,7 +190,8 @@ class HttpRequestHelper { ?CookieJar $cookies = null, bool $stream = false, ?int $timeout = 0, - ?Client $client = null + ?Client $client = null, + ?bool $isGivenStep = false ):ResponseInterface { if ((\getenv('DEBUG_ACCEPTANCE_RESPONSES') !== false) || (\getenv('DEBUG_ACCEPTANCE_API_CALLS') !== false)) { $debugResponses = true; @@ -216,7 +218,7 @@ class HttpRequestHelper { $client ); - if ($response->getStatusCode() >= 400 && $response->getStatusCode() !== self::HTTP_TOO_EARLY) { + if ($response->getStatusCode() >= 400 && $response->getStatusCode() !== self::HTTP_TOO_EARLY && $response->getStatusCode() !== self::HTTP_CONFLICT) { $sendExceptionHappened = true; } @@ -224,11 +226,21 @@ class HttpRequestHelper { self::debugResponse($response); } $sendCount = $sendCount + 1; - $loopAgain = !$sendExceptionHappened && ($response->getStatusCode() === self::HTTP_TOO_EARLY) && ($sendCount <= $sendRetryLimit); + // 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. + // In all these cases we can try the API request again after a short time. + $loopAgain = !$sendExceptionHappened && ($response->getStatusCode() === self::HTTP_TOO_EARLY || + ($response->getStatusCode() === self::HTTP_CONFLICT && $isGivenStep)) && + $sendCount <= $sendRetryLimit; if ($loopAgain) { - // we need to repeat the send request, because we got HTTP_TOO_EARLY + // 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. + self::debugResponse($response); \sleep(1); } } while ($loopAgain); diff --git a/tests/TestHelpers/WebDavHelper.php b/tests/TestHelpers/WebDavHelper.php index 32b44c0a7..a392a8383 100644 --- a/tests/TestHelpers/WebDavHelper.php +++ b/tests/TestHelpers/WebDavHelper.php @@ -601,6 +601,7 @@ class WebDavHelper { * @param Client|null $client * @param array|null $urlParameter to concatenate with path * @param string|null $doDavRequestAsUser run the DAV as this user, if null it is the same as $user + * @param bool $isGivenStep is set to true if makeDavRequest is called from a "given" step * * @return ResponseInterface * @throws GuzzleException @@ -623,7 +624,8 @@ class WebDavHelper { ?int $timeout = 0, ?Client $client = null, ?array $urlParameter = [], - ?string $doDavRequestAsUser = null + ?string $doDavRequestAsUser = null, + ?bool $isGivenStep = false ):ResponseInterface { $baseUrl = self::sanitizeUrl($baseUrl, true); @@ -702,7 +704,8 @@ class WebDavHelper { null, $stream, $timeout, - $client + $client, + $isGivenStep ); } diff --git a/tests/acceptance/features/bootstrap/WebDav.php b/tests/acceptance/features/bootstrap/WebDav.php index 93993edc2..50c669fa4 100644 --- a/tests/acceptance/features/bootstrap/WebDav.php +++ b/tests/acceptance/features/bootstrap/WebDav.php @@ -389,9 +389,12 @@ trait WebDav { * @param string|null $password * @param array|null $urlParameter * @param string|null $doDavRequestAsUser + * @param bool|null $isGivenStep * * @return ResponseInterface - * @throws GuzzleException|JsonException + * + * @throws GuzzleException + * @throws JsonException */ public function makeDavRequest( ?string $user, @@ -404,7 +407,8 @@ trait WebDav { bool $stream = false, ?string $password = null, ?array $urlParameter = [], - ?string $doDavRequestAsUser = null + ?string $doDavRequestAsUser = null, + ?bool $isGivenStep = false ):ResponseInterface { $user = $this->getActualUsername($user); if ($this->customDavPath !== null) { @@ -438,7 +442,8 @@ trait WebDav { $this->httpRequestTimeout, null, $urlParameter, - $doDavRequestAsUser + $doDavRequestAsUser, + $isGivenStep ); } @@ -3520,18 +3525,28 @@ trait WebDav { * * @param string $user * @param string $destination + * @param bool|null $isGivenStep * * @return void * @throws JsonException | GuzzleException + * @throws GuzzleException | JsonException */ - public function userCreatesFolder(string $user, string $destination):void { + public function userCreatesFolder(string $user, string $destination, ?bool $isGivenStep = false):void { $user = $this->getActualUsername($user); $destination = '/' . \ltrim($destination, '/'); $this->response = $this->makeDavRequest( $user, "MKCOL", $destination, - [] + [], + null, + "files", + null, + false, + null, + [], + null, + $isGivenStep ); $this->setResponseXml( HttpRequestHelper::parseResponseAsXml($this->response) @@ -3551,7 +3566,7 @@ trait WebDav { */ public function userHasCreatedFolder(string $user, string $destination):void { $user = $this->getActualUsername($user); - $this->userCreatesFolder($user, $destination); + $this->userCreatesFolder($user, $destination, true); $this->theHTTPStatusCodeShouldBe( ["201", "204"], "HTTP status code was not 201 or 204 while trying to create folder '$destination' for user '$user'"