From 335144e50bd6d1dace7e494bc69a98f391302b69 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Mon, 2 Jan 2023 16:20:53 +0545 Subject: [PATCH 1/5] add tests to check 425 too early behavior add tests to check 425 too early behavior add delay postprocessing suite pipeline add step def --- .drone.star | 10 ++ tests/TestHelpers/HttpRequestHelper.php | 92 +++++++++++++++---- tests/acceptance/config/behat.yml | 16 ++++ ...ected-failures-localAPI-on-OCIS-storage.md | 10 ++ .../delayPostprocessing.feature | 26 ++++++ .../features/bootstrap/AuthContext.php | 30 ++++++ 6 files changed, 165 insertions(+), 19 deletions(-) create mode 100644 tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature diff --git a/.drone.star b/.drone.star index 610b29a852..9c7d0f7252 100644 --- a/.drone.star +++ b/.drone.star @@ -115,6 +115,16 @@ config = { "OCIS_CORS_ALLOW_ORIGINS": "https://aphno.badal", }, }, + "apiDelayPostprocessing": { + "suites": [ + "apiAsyncUpload", + ], + "skip": False, + "earlyFail": True, + "extraServerEnvironment": { + "POSTPROCESSING_DELAY": "30s", + }, + }, }, "apiTests": { "numberOfParts": 10, diff --git a/tests/TestHelpers/HttpRequestHelper.php b/tests/TestHelpers/HttpRequestHelper.php index c6b567b7a4..bf701b6f95 100644 --- a/tests/TestHelpers/HttpRequestHelper.php +++ b/tests/TestHelpers/HttpRequestHelper.php @@ -103,7 +103,7 @@ class HttpRequestHelper { * @return ResponseInterface * @throws GuzzleException */ - public static function sendRequest( + public static function sendRequestOnce( ?string $url, ?string $xRequestId, ?string $method = 'GET', @@ -144,36 +144,90 @@ class HttpRequestHelper { $debugRequests = false; } - if ((\getenv('DEBUG_ACCEPTANCE_RESPONSES') !== false) || (\getenv('DEBUG_ACCEPTANCE_API_CALLS') !== false)) { - $debugResponses = true; - } else { - $debugResponses = false; + // The exceptions that might happen here include: + // ConnectException - in that case there is no response. Don't catch the exception. + // RequestException - if there is something in the response then pass it back. + // otherwise re-throw the exception. + // GuzzleException - something else unexpected happened. Don't catch the exception. + try { + $response = $client->send($request); + } catch (RequestException $ex) { + $response = $ex->getResponse(); + + //if the response was null for some reason do not return it but re-throw + if ($response === null) { + throw $ex; + } } if ($debugRequests) { self::debugRequest($request, $user, $password); } + return $response; + } + + /** + * + * @param string|null $url + * @param string|null $xRequestId + * @param string|null $method + * @param string|null $user + * @param string|null $password + * @param array|null $headers ['X-MyHeader' => 'value'] + * @param mixed $body + * @param array|null $config + * @param CookieJar|null $cookies + * @param bool $stream Set to true to stream a response rather + * than download it all up-front. + * @param int|null $timeout + * @param Client|null $client + * + * @return ResponseInterface + * @throws GuzzleException + */ + public static function sendRequest( + ?string $url, + ?string $xRequestId, + ?string $method = 'GET', + ?string $user = null, + ?string $password = null, + ?array $headers = null, + $body = null, + ?array $config = null, + ?CookieJar $cookies = null, + bool $stream = false, + ?int $timeout = 0, + ?Client $client = null + ):ResponseInterface { + if ((\getenv('DEBUG_ACCEPTANCE_RESPONSES') !== false) || (\getenv('DEBUG_ACCEPTANCE_API_CALLS') !== false)) { + $debugResponses = true; + } else { + $debugResponses = false; + } + $sendRetryLimit = self::numRetriesOnHttpTooEarly(); $sendCount = 0; $sendExceptionHappened = false; do { - // The exceptions that might happen here include: - // ConnectException - in that case there is no response. Don't catch the exception. - // RequestException - if there is something in the response then pass it back. - // otherwise re-throw the exception. - // GuzzleException - something else unexpected happened. Don't catch the exception. - try { - $response = $client->send($request); - } catch (RequestException $ex) { - $sendExceptionHappened = true; - $response = $ex->getResponse(); + $response = self::sendRequestOnce( + $url, + $xRequestId, + $method, + $user, + $password, + $headers, + $body, + $config, + $cookies, + $stream, + $timeout, + $client + ); - //if the response was null for some reason do not return it but re-throw - if ($response === null) { - throw $ex; - } + if ($response->getStatusCode() >= 400) { + $sendExceptionHappened = true; } if ($debugResponses) { diff --git a/tests/acceptance/config/behat.yml b/tests/acceptance/config/behat.yml index 3a9fc84427..41c6d2f417 100644 --- a/tests/acceptance/config/behat.yml +++ b/tests/acceptance/config/behat.yml @@ -152,6 +152,22 @@ default: - OCSContext: - TrashbinContext: + apiAsyncUpload: + paths: + - '%paths.base%/../features/apiAsyncUpload' + context: *common_ldap_suite_context + contexts: + - SpacesContext: + - FeatureContext: *common_feature_context_params + - OccContext: + - WebDavPropertiesContext: + - FavoritesContext: + - ChecksumContext: + - FilesVersionsContext: + - OCSContext: + - TrashbinContext: + + extensions: rdx\behatvars\BehatVariablesExtension: ~ diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index 4ac319d5da..5f10f770c5 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -93,3 +93,13 @@ The expected failures in this file are from features in the owncloud/ocis repo. #### [A User can get information of another user with Graph API](https://github.com/owncloud/ocis/issues/5125) - [apiGraph/getUser.feature:23](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiGraph/getUser.feature#L23) + +#### [GET a file while it's in processing doesn't return 425 code (async uploads)](https://github.com/owncloud/ocis/issues/5326) +- [apiAsyncUpload/delayPostprocessing.feature:14](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L14) +- [apiAsyncUpload/delayPostprocessing.feature:15](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L15) +- [apiAsyncUpload/delayPostprocessing.feature:16](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L16) + +#### [PROPFIND a file while it's in processing doesn't return 425 code (async uploads)](https://github.com/owncloud/ocis/issues/5327) +- [apiAsyncUpload/delayPostprocessing.feature:24](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L24) +- [apiAsyncUpload/delayPostprocessing.feature:25](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L25) +- [apiAsyncUpload/delayPostprocessing.feature:26](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L26) \ No newline at end of file diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature new file mode 100644 index 0000000000..c62148b87b --- /dev/null +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -0,0 +1,26 @@ +@api +Feature: delay post-processing of uploaded files + + Background: + Given user "Alice" has been created with default attributes and without skeleton files + And user "Alice" has uploaded file with content "uploaded content" to "/file.txt" + + + Scenario Outline: user sends GET request to the file while it's still being processed + When user "Alice" requests "" with "GET" without retrying + Then the HTTP status code should be "425" + Examples: + | dav_path | + | /remote.php/webdav/file.txt | + | /remote.php/dav/files/%username%/file.txt | + | /dav/spaces/%spaceid%/file.txt | + + + Scenario Outline: user sends PROPFIND request to the file while it's still being processed + When user "Alice" requests "" with "PROPFIND" without retrying + Then the HTTP status code should be "425" + Examples: + | dav_path | + | /remote.php/webdav/file.txt | + | /remote.php/dav/files/%username%/file.txt | + | /dav/spaces/%spaceid%/file.txt | \ No newline at end of file diff --git a/tests/acceptance/features/bootstrap/AuthContext.php b/tests/acceptance/features/bootstrap/AuthContext.php index 24c57b657d..796eb2bc56 100644 --- a/tests/acceptance/features/bootstrap/AuthContext.php +++ b/tests/acceptance/features/bootstrap/AuthContext.php @@ -1155,4 +1155,34 @@ class AuthContext implements Context { $this->tokenAuthHasBeenSetTo = ''; } } + + /** + * @When user :user requests :endpoint with :method without retrying + * + * @param string $user + * @param string $endpoint + * @param string $method + * + * @return void + */ + public function userRequestsURLWithoutRetry( + string $user, + string $endpoint, + string $method + ):void { + $username = $this->featureContext->getActualUsername($user); + $endpoint = $this->featureContext->substituteInLineCodes( + $endpoint, + $username + ); + $fullUrl = $this->featureContext->getBaseUrl() . $endpoint; + $response = HttpRequestHelper::sendRequestOnce( + $fullUrl, + $this->featureContext->getStepLineRef(), + $method, + $username, + $this->featureContext->getPasswordForUser($user) + ); + $this->featureContext->setResponse($response); + } } From fc69534c735d14aa56b5af157d34d2497aaf665a Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 5 Jan 2023 11:58:30 +0545 Subject: [PATCH 2/5] fix expected failure file --- .../acceptance/expected-failures-localAPI-on-OCIS-storage.md | 5 ++++- .../features/apiAsyncUpload/delayPostprocessing.feature | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index 5f10f770c5..2822e9a165 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -102,4 +102,7 @@ The expected failures in this file are from features in the owncloud/ocis repo. #### [PROPFIND a file while it's in processing doesn't return 425 code (async uploads)](https://github.com/owncloud/ocis/issues/5327) - [apiAsyncUpload/delayPostprocessing.feature:24](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L24) - [apiAsyncUpload/delayPostprocessing.feature:25](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L25) -- [apiAsyncUpload/delayPostprocessing.feature:26](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L26) \ No newline at end of file +- [apiAsyncUpload/delayPostprocessing.feature:26](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L26) + +Note: always have an empty line at the end of this file. +The bash script that processes this file requires that the last line has a newline on the end. diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature index c62148b87b..1d4cace765 100644 --- a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -5,7 +5,7 @@ Feature: delay post-processing of uploaded files Given user "Alice" has been created with default attributes and without skeleton files And user "Alice" has uploaded file with content "uploaded content" to "/file.txt" - + @issue-5326 Scenario Outline: user sends GET request to the file while it's still being processed When user "Alice" requests "" with "GET" without retrying Then the HTTP status code should be "425" @@ -15,7 +15,7 @@ Feature: delay post-processing of uploaded files | /remote.php/dav/files/%username%/file.txt | | /dav/spaces/%spaceid%/file.txt | - + @issue-5327 Scenario Outline: user sends PROPFIND request to the file while it's still being processed When user "Alice" requests "" with "PROPFIND" without retrying Then the HTTP status code should be "425" From d2ebb10339a2b07e4fb2ea09b4a002b776c51b91 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 5 Jan 2023 15:00:32 +0545 Subject: [PATCH 3/5] refactor PROPFIND tests --- .drone.star | 2 +- .../acceptance/expected-failures-localAPI-on-OCIS-storage.md | 5 ----- .../features/apiAsyncUpload/delayPostprocessing.feature | 5 +++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.drone.star b/.drone.star index 9c7d0f7252..645092e4a3 100644 --- a/.drone.star +++ b/.drone.star @@ -115,7 +115,7 @@ config = { "OCIS_CORS_ALLOW_ORIGINS": "https://aphno.badal", }, }, - "apiDelayPostprocessing": { + "apiDelayPostProcessing": { "suites": [ "apiAsyncUpload", ], diff --git a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md index 2822e9a165..45a4d8bdcc 100644 --- a/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-localAPI-on-OCIS-storage.md @@ -99,10 +99,5 @@ The expected failures in this file are from features in the owncloud/ocis repo. - [apiAsyncUpload/delayPostprocessing.feature:15](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L15) - [apiAsyncUpload/delayPostprocessing.feature:16](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L16) -#### [PROPFIND a file while it's in processing doesn't return 425 code (async uploads)](https://github.com/owncloud/ocis/issues/5327) -- [apiAsyncUpload/delayPostprocessing.feature:24](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L24) -- [apiAsyncUpload/delayPostprocessing.feature:25](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L25) -- [apiAsyncUpload/delayPostprocessing.feature:26](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature#L26) - Note: always have an empty line at the end of this file. The bash script that processes this file requires that the last line has a newline on the end. diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature index 1d4cace765..21584686ab 100644 --- a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -15,10 +15,11 @@ Feature: delay post-processing of uploaded files | /remote.php/dav/files/%username%/file.txt | | /dav/spaces/%spaceid%/file.txt | - @issue-5327 + Scenario Outline: user sends PROPFIND request to the file while it's still being processed When user "Alice" requests "" with "PROPFIND" without retrying - Then the HTTP status code should be "425" + Then the HTTP status code should be "207" + And the value of the item "//d:response/d:propstat/d:status" in the response should be "HTTP/1.1 425 TOO EARLY" Examples: | dav_path | | /remote.php/webdav/file.txt | From aae418c2ab6e674dcd4959decb530f4f7bfa50f7 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Thu, 5 Jan 2023 17:54:33 +0545 Subject: [PATCH 4/5] add test to PROPFIND folder --- .../delayPostprocessing.feature | 19 +++++++++++++++-- .../bootstrap/WebDavPropertiesContext.php | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature index 21584686ab..8176ea70e0 100644 --- a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -3,10 +3,10 @@ Feature: delay post-processing of uploaded files Background: Given user "Alice" has been created with default attributes and without skeleton files - And user "Alice" has uploaded file with content "uploaded content" to "/file.txt" @issue-5326 Scenario Outline: user sends GET request to the file while it's still being processed + Given user "Alice" has uploaded file with content "uploaded content" to "/file.txt" When user "Alice" requests "" with "GET" without retrying Then the HTTP status code should be "425" Examples: @@ -17,6 +17,7 @@ Feature: delay post-processing of uploaded files Scenario Outline: user sends PROPFIND request to the file while it's still being processed + Given user "Alice" has uploaded file with content "uploaded content" to "/file.txt" When user "Alice" requests "" with "PROPFIND" without retrying Then the HTTP status code should be "207" And the value of the item "//d:response/d:propstat/d:status" in the response should be "HTTP/1.1 425 TOO EARLY" @@ -24,4 +25,18 @@ Feature: delay post-processing of uploaded files | dav_path | | /remote.php/webdav/file.txt | | /remote.php/dav/files/%username%/file.txt | - | /dav/spaces/%spaceid%/file.txt | \ No newline at end of file + | /dav/spaces/%spaceid%/file.txt | + + + Scenario Outline: user sends PROPFIND request to the folder while its some files are still being processed + Given user "Alice" has created folder "my_data" + And user "Alice" has uploaded file with content "uploaded content" to "/my_data/file.txt" + When user "Alice" requests "" with "PROPFIND" without retrying + Then the HTTP status code should be "207" + And as user "Alice" the value of the item "//d:status" of path "/" in the response should be "HTTP/1.1 200 OK" + And as user "Alice" the value of the item "//d:status" of path "/file.txt" in the response should be "HTTP/1.1 425 TOO EARLY" + Examples: + | dav_path | + | /remote.php/webdav/my_data | + | /remote.php/dav/files/%username%/my_data | + | /dav/spaces/%spaceid%/my_data | \ No newline at end of file diff --git a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php index 62275ef27b..6aa673e1b0 100644 --- a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php +++ b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php @@ -644,6 +644,27 @@ class WebDavPropertiesContext implements Context { ); } + /** + * @Then as user :user the value of the item :xpath of path :path in the response should be :value + * + * @param string $user + * @param string $xpath + * @param string $path + * @param string $expectedValue + * + * @return void + * @throws Exception + */ + public function valueOfItemOfPathShouldBe(string $user, string $xpath, string $path, string $expectedValue):void { + $path = $this->featureContext->substituteInLineCodes($path, $user); + $fullXpath = "//d:response/d:href[.='$path']/following-sibling::d:propstat$xpath"; + $this->assertValueOfItemInResponseAboutUserIs( + $fullXpath, + null, + $expectedValue + ); + } + /** * @Then the value of the item :xpath in the response about user :user should be :value * From 6e68fae2c3813977164aac8580a9677a926d5bc5 Mon Sep 17 00:00:00 2001 From: Saw-jan Date: Fri, 6 Jan 2023 12:21:19 +0545 Subject: [PATCH 5/5] fix sendException logic --- tests/TestHelpers/HttpRequestHelper.php | 2 +- .../features/apiAsyncUpload/delayPostprocessing.feature | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TestHelpers/HttpRequestHelper.php b/tests/TestHelpers/HttpRequestHelper.php index bf701b6f95..2348e7d6be 100644 --- a/tests/TestHelpers/HttpRequestHelper.php +++ b/tests/TestHelpers/HttpRequestHelper.php @@ -226,7 +226,7 @@ class HttpRequestHelper { $client ); - if ($response->getStatusCode() >= 400) { + if ($response->getStatusCode() >= 400 && $response->getStatusCode() !== self::HTTP_TOO_EARLY) { $sendExceptionHappened = true; } diff --git a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature index 8176ea70e0..f50b239268 100644 --- a/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature +++ b/tests/acceptance/features/apiAsyncUpload/delayPostprocessing.feature @@ -28,7 +28,7 @@ Feature: delay post-processing of uploaded files | /dav/spaces/%spaceid%/file.txt | - Scenario Outline: user sends PROPFIND request to the folder while its some files are still being processed + Scenario Outline: user sends PROPFIND request to the folder while files in the folder are still being processed Given user "Alice" has created folder "my_data" And user "Alice" has uploaded file with content "uploaded content" to "/my_data/file.txt" When user "Alice" requests "" with "PROPFIND" without retrying