From db2f7b6f3798beec64287c974cb6557c4e78d9eb Mon Sep 17 00:00:00 2001 From: nabim777 Date: Tue, 7 Jan 2025 14:56:37 +0545 Subject: [PATCH] review resolve Signed-off-by: nabim777 --- tests/acceptance/TestHelpers/TusClient.php | 19 +------------ .../acceptance/bootstrap/SpacesTUSContext.php | 6 +++-- tests/acceptance/bootstrap/TUSContext.php | 3 ++- .../expected-failures-without-remotephp.md | 4 +-- .../acceptance/features/apiOcm/share.feature | 2 ++ .../features/apiSpaces/tusUpload.feature | 27 ++++++++++++------- 6 files changed, 29 insertions(+), 32 deletions(-) diff --git a/tests/acceptance/TestHelpers/TusClient.php b/tests/acceptance/TestHelpers/TusClient.php index af5e384cb27..888d121d75d 100644 --- a/tests/acceptance/TestHelpers/TusClient.php +++ b/tests/acceptance/TestHelpers/TusClient.php @@ -24,7 +24,6 @@ use Carbon\Carbon; use GuzzleHttp\Exception\ClientException; -use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Exception\GuzzleException; use PHPUnit\Framework\Assert; use TusPhp\Exception\ConnectionException; @@ -55,7 +54,6 @@ public function createUploadWithResponse(string $key, int $bytes = -1): Response 'Upload-Checksum' => $this->getUploadChecksumHeader(), 'Upload-Metadata' => $this->getUploadMetadataHeader(), ]; - $data = ''; if ($bytes > 0) { $data = $this->getData(0, $bytes); @@ -65,11 +63,9 @@ public function createUploadWithResponse(string $key, int $bytes = -1): Response 'Content-Length' => \strlen($data), ]; } - if ($this->isPartial()) { $headers += ['Upload-Concat' => 'partial']; } - try { $response = $this->getClient()->post( $this->apiPath, @@ -81,15 +77,11 @@ public function createUploadWithResponse(string $key, int $bytes = -1): Response } catch (ClientException $e) { $response = $e->getResponse(); } - $statusCode = $response->getStatusCode(); - if ($statusCode !== HttpResponse::HTTP_CREATED) { return $response; } - $uploadLocation = current($response->getHeader('location')); - $this->getCache()->set( $this->getKey(), [ @@ -105,13 +97,11 @@ public function createUploadWithResponse(string $key, int $bytes = -1): Response * * @return ResponseInterface * @throws GuzzleException - * @throws ConnectionException - * @throws TusException + * @throws TusException | ConnectionException */ public function uploadWithResponse(int $bytes = -1): ResponseInterface { $bytes = $bytes < 0 ? $this->getFileSize() : $bytes; $offset = $this->partialOffset < 0 ? 0 : $this->partialOffset; - try { // Check if this upload exists with HEAD request. $offset = $this->sendHeadRequest(); @@ -121,22 +111,17 @@ public function uploadWithResponse(int $bytes = -1): ResponseInterface { if ($this->url->getStatusCode() !== HttpResponse::HTTP_CREATED) { return $this->url; } - } catch (ConnectException) { - throw new ConnectionException("Couldn't connect to server."); } - // Verify that upload is not yet expired. if ($this->isExpired()) { throw new TusException('Upload expired.'); } - $data = $this->getData($offset, $bytes); $headers = $this->headers + [ 'Content-Type' => self::HEADER_CONTENT_TYPE, 'Content-Length' => \strlen($data), 'Upload-Checksum' => $this->getUploadChecksumHeader(), ]; - if ($this->isPartial()) { $headers += ['Upload-Concat' => self::UPLOAD_TYPE_PARTIAL]; } else { @@ -152,8 +137,6 @@ public function uploadWithResponse(int $bytes = -1): ResponseInterface { ); } catch (ClientException $e) { throw $this->handleClientException($e); - } catch (ConnectException) { - throw new ConnectionException("Couldn't connect to server."); } return $response; } diff --git a/tests/acceptance/bootstrap/SpacesTUSContext.php b/tests/acceptance/bootstrap/SpacesTUSContext.php index 906f6251972..b0a22257b5c 100644 --- a/tests/acceptance/bootstrap/SpacesTUSContext.php +++ b/tests/acceptance/bootstrap/SpacesTUSContext.php @@ -97,8 +97,9 @@ public function userUploadsAFileViaTusInsideOfTheSpaceUsingTheWebdavApi( string $spaceName ): void { $spaceId = $this->spacesContext->getSpaceIdByName($user, $spaceName); - $this->tusContext->uploadFileUsingTus($user, $source, $destination, $spaceId); + $response = $this->tusContext->uploadFileUsingTus($user, $source, $destination, $spaceId); $this->featureContext->setLastUploadDeleteTime(\time()); + $this->featureContext->setResponse($response); } /** @@ -217,7 +218,7 @@ public function userUploadsAFileWithContentToInsideFederatedShareViaTusUsingTheW $remoteItemId = $this->spacesContext->getSharesRemoteItemId($user, $destination); $remoteItemId = \rawurlencode($remoteItemId); $tmpFile = $this->tusContext->writeDataToTempFile($content); - $this->tusContext->uploadFileUsingTus( + $response = $this->tusContext->uploadFileUsingTus( $user, \basename($tmpFile), $file, @@ -225,6 +226,7 @@ public function userUploadsAFileWithContentToInsideFederatedShareViaTusUsingTheW ); $this->featureContext->setLastUploadDeleteTime(\time()); \unlink($tmpFile); + $this->featureContext->setResponse($response); } /** diff --git a/tests/acceptance/bootstrap/TUSContext.php b/tests/acceptance/bootstrap/TUSContext.php index ec83f12cede..803cda2a25a 100644 --- a/tests/acceptance/bootstrap/TUSContext.php +++ b/tests/acceptance/bootstrap/TUSContext.php @@ -265,8 +265,9 @@ public function userUploadsUsingTusAFileTo( int $bytes = null, string $checksum = '' ): void { - $this->uploadFileUsingTus($user, $source, $destination, null, $uploadMetadata, $noOfChunks, $bytes, $checksum); + $response = $this->uploadFileUsingTus($user, $source, $destination, null, $uploadMetadata, $noOfChunks, $bytes, $checksum); $this->featureContext->setLastUploadDeleteTime(\time()); + $this->featureContext->setResponse($response); } /** diff --git a/tests/acceptance/expected-failures-without-remotephp.md b/tests/acceptance/expected-failures-without-remotephp.md index 6ef236ad12d..0f48f327cbf 100644 --- a/tests/acceptance/expected-failures-without-remotephp.md +++ b/tests/acceptance/expected-failures-without-remotephp.md @@ -323,8 +323,8 @@ #### [Cannot create new TUS upload resource using /webdav without remote.php - returns html instead](https://github.com/owncloud/ocis/issues/10346) -- [apiSpaces/tusUpload.feature:60](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpaces/tusUpload.feature#L60) -- [apiSpaces/tusUpload.feature:104](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpaces/tusUpload.feature#L104) +- [apiSpaces/tusUpload.feature:63](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpaces/tusUpload.feature#L63) +- [apiSpaces/tusUpload.feature:110](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpaces/tusUpload.feature#L110) - [coreApiWebdavUploadTUS/creationWithUploadExtension.feature:38](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavUploadTUS/creationWithUploadExtension.feature#L38) - [coreApiWebdavUploadTUS/uploadFile.feature:16](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavUploadTUS/uploadFile.feature#L16) - [coreApiWebdavUploadTUS/uploadFile.feature:17](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavUploadTUS/uploadFile.feature#L17) diff --git a/tests/acceptance/features/apiOcm/share.feature b/tests/acceptance/features/apiOcm/share.feature index 43266cd7714..e9f50b0c44a 100755 --- a/tests/acceptance/features/apiOcm/share.feature +++ b/tests/acceptance/features/apiOcm/share.feature @@ -1048,6 +1048,7 @@ Feature: an user shares resources using ScienceMesh application | permissionsRole | Editor | When using server "REMOTE" And user "Brian" uploads a file with content "lorem" to "file.txt" inside federated share "FOLDER" via TUS using the WebDAV API + And the HTTP status code should be "204" Then for user "Brian" the content of file "file.txt" of federated share "FOLDER" should be "lorem" @issue-10285 @issue-10536 @@ -1066,6 +1067,7 @@ Feature: an user shares resources using ScienceMesh application | permissionsRole | Editor | When using server "LOCAL" And user "Alice" uploads a file with content "lorem" to "file.txt" inside federated share "FOLDER" via TUS using the WebDAV API + And the HTTP status code should be "204" Then for user "Alice" the content of file "file.txt" of federated share "FOLDER" should be "lorem" @issue-10495 diff --git a/tests/acceptance/features/apiSpaces/tusUpload.feature b/tests/acceptance/features/apiSpaces/tusUpload.feature index 34f561b841e..047a9cf6e96 100644 --- a/tests/acceptance/features/apiSpaces/tusUpload.feature +++ b/tests/acceptance/features/apiSpaces/tusUpload.feature @@ -14,7 +14,8 @@ Feature: upload resources using TUS protocol Scenario: upload a file within the set quota to a project space Given user "Alice" has created a space "Project Jupiter" of type "project" with quota "10000" When user "Alice" uploads a file with content "uploaded content" to "/upload.txt" via TUS inside of the space "Project Jupiter" using the WebDAV API - Then for user "Alice" the space "Project Jupiter" should contain these entries: + Then the HTTP status code should be "204" + And for user "Alice" the space "Project Jupiter" should contain these entries: | upload.txt | @@ -36,7 +37,8 @@ Feature: upload resources using TUS protocol Given user "Alice" has uploaded a file with content "uploaded content" to "/upload.txt" via TUS inside of the space "Alice Hansen" And user "Alice" has moved file "upload.txt" to "test.txt" in space "Alice Hansen" When user "Alice" uploads a file with content "uploaded content" to "/upload.txt" via TUS inside of the space "Alice Hansen" using the WebDAV API - Then for user "Alice" the space "Alice Hansen" should contain these entries: + Then the HTTP status code should be "204" + And for user "Alice" the space "Alice Hansen" should contain these entries: | test.txt | | upload.txt | @@ -53,7 +55,8 @@ Feature: upload resources using TUS protocol | permissionsRole | Editor | And user "Brian" has a share "testFolder" synced When user "Brian" uploads file "filesForUpload/zerobyte.txt" to "Shares/testFolder/textfile.txt" using the TUS protocol on the WebDAV API - Then the content of file "Shares/testFolder/textfile.txt" for user "Brian" should be "" + Then the HTTP status code should be "201" + And the content of file "Shares/testFolder/textfile.txt" for user "Brian" should be "" And the content of file "testFolder/textfile.txt" for user "Alice" should be "" Examples: | dav-path-version | @@ -73,7 +76,8 @@ Feature: upload resources using TUS protocol | permissionsRole | Editor | And user "Brian" has a share "testFolder" synced When user "Brian" uploads a file from "filesForUpload/zerobyte.txt" to "testFolder/textfile.txt" via TUS inside of the space "Shares" using the WebDAV API - Then for user "Brian" the content of the file "testFolder/textfile.txt" of the space "Shares" should be "" + Then the HTTP status code should be "201" + And for user "Brian" the content of the file "testFolder/textfile.txt" of the space "Shares" should be "" And for user "Alice" the content of the file "testFolder/textfile.txt" of the space "Personal" should be "" @@ -82,7 +86,8 @@ Feature: upload resources using TUS protocol And the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API And user "Alice" has created a space "new-space" with the default quota using the Graph API When user "Alice" uploads a file from "filesForUpload/zerobyte.txt" to "textfile.txt" via TUS inside of the space "new-space" using the WebDAV API - Then for user "Alice" the content of the file "textfile.txt" of the space "new-space" should be "" + Then the HTTP status code should be "201" + And for user "Alice" the content of the file "textfile.txt" of the space "new-space" should be "" @issue-8003 @issue-10346 Scenario Outline: replace a shared file with zero-byte file @@ -97,7 +102,8 @@ Feature: upload resources using TUS protocol | permissionsRole | File Editor | And user "Brian" has a share "textfile.txt" synced When user "Brian" uploads file "filesForUpload/zerobyte.txt" to "Shares/textfile.txt" using the TUS protocol on the WebDAV API - Then the content of file "Shares/textfile.txt" for user "Brian" should be "" + Then the HTTP status code should be "201" + And the content of file "Shares/textfile.txt" for user "Brian" should be "" And the content of file "textfile.txt" for user "Alice" should be "" Examples: | dav-path-version | @@ -117,7 +123,8 @@ Feature: upload resources using TUS protocol | permissionsRole | File Editor | And user "Brian" has a share "textfile.txt" synced When user "Brian" uploads a file from "filesForUpload/zerobyte.txt" to "textfile.txt" via TUS inside of the space "Shares" using the WebDAV API - Then for user "Brian" the content of the file "textfile.txt" of the space "Shares" should be "" + Then the HTTP status code should be "201" + And for user "Brian" the content of the file "textfile.txt" of the space "Shares" should be "" And for user "Alice" the content of the file "textfile.txt" of the space "Personal" should be "" @issue-8003 @@ -127,7 +134,8 @@ Feature: upload resources using TUS protocol And user "Alice" has created a space "new-space" with the default quota using the Graph API And user "Alice" has uploaded a file inside space "new-space" with content "This is TUS upload" to "textfile.txt" When user "Alice" uploads a file from "filesForUpload/zerobyte.txt" to "textfile.txt" via TUS inside of the space "new-space" using the WebDAV API - Then for user "Alice" the content of the file "textfile.txt" of the space "new-space" should be "" + Then the HTTP status code should be "201" + And for user "Alice" the content of the file "textfile.txt" of the space "new-space" should be "" @issue-8003 Scenario: replace a file inside a shared project space with zero-byte file @@ -142,5 +150,6 @@ Feature: upload resources using TUS protocol | shareType | user | | permissionsRole | Space Editor | When user "Brian" uploads a file from "filesForUpload/zerobyte.txt" to "textfile.txt" via TUS inside of the space "new-space" using the WebDAV API - Then for user "Brian" the content of the file "textfile.txt" of the space "new-space" should be "" + Then the HTTP status code should be "201" + And for user "Brian" the content of the file "textfile.txt" of the space "new-space" should be "" And for user "Alice" the content of the file "textfile.txt" of the space "new-space" should be ""