From a88dbdc97b716b04c4e207d49489c0ae243e4d71 Mon Sep 17 00:00:00 2001 From: Swikriti Tripathi <41103328+SwikritiT@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:03:44 +0545 Subject: [PATCH] Remove the trashed state from fileId endpoint (#554) * Remove the trashed state from fileId endpoint Signed-off-by: Swikriti Tripathi * refactor tests Signed-off-by: Swikriti Tripathi * fix unit tests Signed-off-by: Swikriti Tripathi * refactor tests Signed-off-by: Swikriti Tripathi * refactor tests Signed-off-by: Swikriti Tripathi * Address reviews Signed-off-by: Swikriti Tripathi --------- Signed-off-by: Swikriti Tripathi --- lib/Controller/FilesController.php | 53 +---- .../api/getFileinfoByFileIDAPI.feature | 212 ++++++++++++----- .../api/getFilesinfoByFileIDsAPI.feature | 56 ++--- tests/lib/Controller/FilesControllerTest.php | 222 +++--------------- 4 files changed, 204 insertions(+), 339 deletions(-) diff --git a/lib/Controller/FilesController.php b/lib/Controller/FilesController.php index 6fa367e48..9538c5ccc 100644 --- a/lib/Controller/FilesController.php +++ b/lib/Controller/FilesController.php @@ -11,14 +11,10 @@ namespace OCA\OpenProject\Controller; -use Exception; use OCA\Activity\Data; use OCA\Activity\GroupHelperDisabled; use OCA\Activity\UserSettings; -use OCA\Files_Trashbin\Trash\ITrashManager; use OCP\Activity\IManager; -use OCP\App\IAppManager; -use OCP\AppFramework\QueryException; use OCP\Files\Config\ICachedMountFileInfo; use OCP\Files\Config\IMountProviderCollection; use OCP\Files\FileInfo; @@ -33,7 +29,6 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Server; -use Throwable; use OCP\Files\DavUtil; class FilesController extends OCSController { @@ -67,14 +62,6 @@ class FilesController extends OCSController { * @var IUserManager */ private $userManager; - /** - * @var IAppManager - */ - private $appManager; - /** - * @var ITrashManager - */ - private $trashManager = null; /** * @var DavUtil */ @@ -87,7 +74,6 @@ class FilesController extends OCSController { * @param IUserSession $userSession * @param IMountProviderCollection $mountCollection * @param IManager $activityManager - * @param IAppManager $appManager * @param IDBConnection $connection * @param ILogger $logger * @param IUserManager $userManager @@ -100,7 +86,6 @@ public function __construct(string $appName, IUserSession $userSession, IMountProviderCollection $mountCollection, IManager $activityManager, - IAppManager $appManager, IDBConnection $connection, ILogger $logger, IUserManager $userManager, @@ -114,7 +99,6 @@ public function __construct(string $appName, $this->connection = $connection; $this->logger = $logger; $this->userManager = $userManager; - $this->appManager = $appManager; $this->davUtils = $davUtils; } @@ -155,20 +139,6 @@ public function getFilesInfo(?array $fileIds): DataResponse { return new DataResponse($result); } - /** - * Function to make the trash-manager testable - * - * @param ITrashManager|null $trashManager - * @return void - * @throws QueryException - */ - public function setTrashManager(ITrashManager $trashManager = null): void { - if ($trashManager !== null) { - $this->trashManager = $trashManager; - } elseif ($this->trashManager === null) { - $this->trashManager = \OC::$server->get(ITrashManager::class); - } - } /** * @param int $fileId * @return array @@ -178,24 +148,12 @@ public function setTrashManager(ITrashManager $trashManager = null): void { */ private function compileFileInfo($fileId) { $file = null; - $trashed = false; $userFolder = $this->rootFolder->getUserFolder($this->user->getUID()); $files = $userFolder->getById($fileId); if (is_array($files) && count($files) > 0) { $file = $files[0]; - } elseif ($this->appManager->isEnabledForUser('files_trashbin')) { - try { - $this->setTrashManager(); - $file = $this->trashManager->getTrashNodeById( - $this->user, $fileId - ); - $trashed = true; - } catch (Exception | Throwable $e) { - $this->logger->error('failed to use the trashManager', ['exception' => $e]); - } } - $mounts = $this->mountCollection->getMountCache()->getMountsForFileId($fileId); if ($file !== null && is_array($mounts) && count($mounts) > 0) { @@ -249,7 +207,6 @@ private function compileFileInfo($fileId) { 'size' => $file->getSize(), 'owner_name' => $owner->getDisplayName(), 'owner_id' => $owner->getUID(), - 'trashed' => $trashed, 'modifier_name' => $modifierName, 'modifier_id' => $modifierId, 'dav_permissions' => $davPermission, @@ -258,6 +215,16 @@ private function compileFileInfo($fileId) { } if (is_array($mounts) && count($mounts) > 0) { + // if the file is in trashbin send 404 + foreach ($mounts as $mount) { + if (str_starts_with($mount->getInternalPath(), 'files_trashbin')) { + return [ + 'status' => 'Not Found', + 'statuscode' => 404, + ]; + } + } + // if the file is of another user send 403 return [ 'status' => 'Forbidden', 'statuscode' => 403, diff --git a/tests/acceptance/features/api/getFileinfoByFileIDAPI.feature b/tests/acceptance/features/api/getFileinfoByFileIDAPI.feature index de8f19899..00dcf9173 100644 --- a/tests/acceptance/features/api/getFileinfoByFileIDAPI.feature +++ b/tests/acceptance/features/api/getFileinfoByFileIDAPI.feature @@ -22,10 +22,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -39,7 +43,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RGDNVW$"}, "path": {"type": "string", "pattern":"^files/file.txt$"} } @@ -69,10 +72,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -86,7 +93,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RGDNVW"}, "path": {"type": "string", "pattern":"^files\/subfolder\/file.txt$"} } @@ -98,14 +104,17 @@ Feature: retrieve file information of a single file, using the file ID And user "Carol" has uploaded file with content "some data" to "file.txt" And user "Carol" has deleted file "file.txt" When user "Carol" gets the information of last created file - Then the HTTP status code should be "200" + Then the HTTP status code should be "404" And the ocs data of the response should match """" { "type": "object", "required": [ "status", - "statuscode", + "statuscode" + ], + "not": { + "required": [ "id", "size", "name", @@ -116,26 +125,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", - "path" - ], + "path", + "trashed" + ] + }, "properties": { - "status": {"type": "string", "pattern": "^OK$"}, - "statuscode" : {"type" : "number", "enum": [200]}, - "id" : {"type" : "integer", "minimum": 1, "maximum": 99999}, - "size" : {"type" : "integer", "enum": [9] }, - "mtime" : {"type" : "integer"}, - "ctime" : {"type" : "integer", "enum": [0]}, - "name": {"type": "string", "pattern": "^file.txt.d\\d{10}$"}, - "mimetype": {"type": "string", "pattern": "^text\/plain$"}, - "owner_id": {"type": "string", "pattern": "^Carol$"}, - "owner_name": {"type": "string", "pattern": "^Carol$"}, - "modifier_id": {"type": "null"}, - "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [true]}, - "dav_permissions": {"type": "string", "pattern":"^RGDNVW"}, - "path": {"type": "string", "pattern":"^files_trashbin\/files\/file.txt.d\\d{10}$"} + "status": {"type": "string", "pattern": "^Not Found$"}, + "statuscode" : {"type" : "number", "enum": [404]} } } """ @@ -146,14 +143,17 @@ Feature: retrieve file information of a single file, using the file ID And user "Carol" has uploaded file with content "some data" to "/subfolder/file.txt" And user "Carol" has deleted folder "subfolder" When user "Carol" gets the information of last created file - Then the HTTP status code should be "200" + Then the HTTP status code should be "404" And the ocs data of the response should match """" { "type": "object", "required": [ "status", - "statuscode", + "statuscode" + ], + "not": { + "required": [ "id", "size", "name", @@ -164,26 +164,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", - "path" - ], + "path", + "trashed" + ] + }, "properties": { - "status": {"type": "string", "pattern": "^OK$"}, - "statuscode" : {"type" : "number", "enum": [200]}, - "id" : {"type" : "integer", "minimum": 1, "maximum": 99999}, - "size" : {"type" : "integer", "enum": [9] }, - "mtime" : {"type" : "integer"}, - "ctime" : {"type" : "integer", "enum": [0]}, - "name": {"type": "string", "pattern": "^file.txt$"}, - "mimetype": {"type": "string", "pattern": "^text\/plain$"}, - "owner_id": {"type": "string", "pattern": "^Carol$"}, - "owner_name": {"type": "string", "pattern": "^Carol$"}, - "modifier_id": {"type": "null"}, - "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [true]}, - "dav_permissions": {"type": "string", "pattern":"^RGDNVW$"}, - "path": {"type": "string", "pattern":"^files_trashbin\/files\/subfolder.d\\d{10}\/file.txt"} + "status": {"type": "string", "pattern": "^Not Found$"}, + "statuscode" : {"type" : "number", "enum": [404]} } } """ @@ -214,9 +202,9 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", - "path" + "path", + "trashed" ] }, "properties": { @@ -250,9 +238,9 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", - "path" + "path", + "trashed" ] }, "properties": { @@ -281,10 +269,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -293,7 +285,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGNVW$"}, "path": {"type": "string", "pattern":"^files/file.txt$"} } @@ -320,10 +311,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -332,7 +327,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGDNVW$"}, "path": {"type": "string", "pattern":"^files\/to-share\/file.txt$"} } @@ -360,10 +354,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -372,7 +370,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGDNVW$"}, "path": {"type": "string", "pattern":"^files\/to-share\/file.txt$"} } @@ -399,10 +396,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -411,7 +412,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGNVW$"}, "path": {"type": "string", "pattern":"^files\/renamed.txt$"} } @@ -439,10 +439,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -451,7 +455,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGDNVW$"}, "path": {"type": "string", "pattern":"^files\/to-share\/renamed.txt$"} } @@ -482,6 +485,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200] }, @@ -517,9 +525,9 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", - "path" + "path", + "trashed" ] }, "properties": { @@ -553,6 +561,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -599,6 +612,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -660,6 +678,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -710,6 +733,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -757,6 +785,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -803,6 +836,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -840,6 +878,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -881,10 +924,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -898,7 +945,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol Hansen$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RGDNVCK$"}, "path": {"type": "string", "pattern":"^files\/folder\/$"} } @@ -931,10 +977,14 @@ Feature: retrieve file information of a single file, using the file ID "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -948,7 +998,6 @@ Feature: retrieve file information of a single file, using the file ID "owner_name": {"type": "string", "pattern": "^Carol Hansen$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RGDNVW$"}, "path": {"type": "string", "pattern":"^files\/folder\/file.txt$"} } @@ -981,6 +1030,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1044,6 +1098,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1082,6 +1141,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1114,6 +1178,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1163,6 +1232,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1195,6 +1269,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, @@ -1250,6 +1329,11 @@ Feature: retrieve file information of a single file, using the file ID "dav_permissions", "path" ], + "not": { + "required": [ + "trashed" + ] + }, "properties": { "status": {"type": "string", "pattern": "^OK$"}, "statuscode" : {"type" : "number", "enum": [200]}, diff --git a/tests/acceptance/features/api/getFilesinfoByFileIDsAPI.feature b/tests/acceptance/features/api/getFilesinfoByFileIDsAPI.feature index 8d51f93d3..08ee5ba72 100644 --- a/tests/acceptance/features/api/getFilesinfoByFileIDsAPI.feature +++ b/tests/acceptance/features/api/getFilesinfoByFileIDsAPI.feature @@ -47,7 +47,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], @@ -64,7 +63,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RGDNVW$"}, "path": {"type": "string", "pattern":"^files\/file.txt$"} } @@ -84,7 +82,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], @@ -101,7 +98,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name": {"type": "string", "pattern": "^Brian Adams$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^SRGNVW$"}, "path": {"type": "string", "pattern":"^files\/renamedByCarol.txt$"} } @@ -110,37 +106,27 @@ Feature: retrieve information of multiple files using the file IDs "type": "object", "required": [ "status", - "statuscode", - "id", - "size", - "name", - "mtime", - "ctime", - "mimetype", - "owner_id", - "owner_name", - "modifier_id", - "modifier_name", - "trashed", - "dav_permissions", - "path" + "statuscode" ], + "not": { + "required": [ + "id", + "size", + "name", + "mtime", + "ctime", + "mimetype", + "owner_id", + "owner_name", + "modifier_id", + "modifier_name", + "dav_permissions", + "path" + ] + }, "properties": { - "status": {"type": "string", "pattern": "^OK$"}, - "statuscode" : {"type" : "number", "enum": [200]}, - "id" : {"type" : "integer", "minimum": 1, "maximum": 99999}, - "size" : {"type" : "integer", "enum": [9] }, - "mtime" : {"type" : "integer"}, - "ctime" : {"type" : "integer", "enum": [0]}, - "name": {"type": "string", "pattern": "^trashed.txt.d\\d{10}$"}, - "mimetype": {"type": "string", "pattern": "^text\/plain$"}, - "owner_id": {"type": "string", "pattern": "^Carol$"}, - "owner_name": {"type": "string", "pattern": "^Carol$"}, - "modifier_id": {"type": "null"}, - "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [true]}, - "dav_permissions": {"type": "string", "pattern":"^RGDNVW$"}, - "path": {"type": "string", "pattern":"^files_trashbin\/files\/trashed.txt.d\\d{10}$"} + "status": {"type": "string", "pattern": "^Not Found$"}, + "statuscode" : {"type" : "number", "enum": [404]} } }, "%ids[3]%": { @@ -161,7 +147,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ] @@ -189,7 +174,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ] @@ -214,7 +198,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name", "modifier_id", "modifier_name", - "trashed", "dav_permissions", "path" ], @@ -231,7 +214,6 @@ Feature: retrieve information of multiple files using the file IDs "owner_name": {"type": "string", "pattern": "^Carol$"}, "modifier_id": {"type": "null"}, "modifier_name": {"type": "null"}, - "trashed": {"type": "boolean", "enum": [false]}, "dav_permissions": {"type": "string", "pattern":"^RMGDNVCK"}, "path": {"type": "string", "pattern":"^files/groupFolder\/$"} } diff --git a/tests/lib/Controller/FilesControllerTest.php b/tests/lib/Controller/FilesControllerTest.php index 558991dfc..9416908de 100644 --- a/tests/lib/Controller/FilesControllerTest.php +++ b/tests/lib/Controller/FilesControllerTest.php @@ -2,7 +2,6 @@ namespace OCA\OpenProject\Controller; -use OCA\Files_Trashbin\Trash\ITrashManager; use OCP\Activity\IManager; use OCP\Files\Config\ICachedMountFileInfo; use OCP\Files\DavUtil; @@ -126,7 +125,6 @@ public function testGetFileInfo( "size" => 200245, "owner_name" => "Test User", "owner_id" => "3df8ff78-49cb-4d60-8d8b-171b29591fd3", - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVCK', @@ -148,80 +146,9 @@ public function testGetFileInfoFileNotFound(): void { assertSame(404, $result->getStatus()); } - public function testGetFileInfoFileInTrash(): void { - $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); - $folderMock->method('getById')->willReturn([]); - - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById')->willReturn( - $this->getNodeMock('text/plain', 759, 'file', '/testUser/files_trashbin/files/welcome.txt.d1648724302') - ); - - $mountCacheMock = $this->getSimpleMountCacheMock( - 'files_trashbin/files/welcome.txt.d1648724302' - ); - $filesController = $this->getFilesControllerMock( - ['getDavPermissions'], $folderMock, $mountCacheMock, true, null, $trashManagerMock - ); - $filesController->method('getDavPermissions')->willReturn('RGDNVW'); - - $result = $filesController->getFileInfo(759); - assertSame($this->trashedWelcomeTxtResult, $result->getData()); - assertSame(200, $result->getStatus()); - } - - // this case happens for files that have been deleted before the trashbinapp got disabled - public function testGetFileInfoFileFileExistsTrashappDisabled(): void { - $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); - $folderMock->method('getById')->willReturn([]); - $mountCacheMock = $this->getSimpleMountCacheMock( - 'files_trashbin/files/welcome.txt.d1648724302' - ); - $filesController = $this->createFilesController( - $folderMock, null, $mountCacheMock, false - ); - - $result = $filesController->getFileInfo(123); - assertSame($this->forbiddenResponse, $result->getData()); - assertSame(403, $result->getStatus()); - } - - // this case happens for files that get deleted while the trashbinapp was disabled - public function testGetFileInfoFileFileDoesNotExistsTrashappDisabled(): void { - $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); - $folderMock->method('getById')->willReturn([]); - $filesController = $this->createFilesController( - $folderMock, null, null, false - ); - - $result = $filesController->getFileInfo(123); - assertSame($this->notFoundResponse, $result->getData()); - assertSame(404, $result->getStatus()); - } - - public function testGetFileInfoFileTrashappThrowsException(): void { - $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); - $folderMock->method('getById')->willReturn([]); - - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById')->willThrowException(new \Exception()); - - $filesController = $this->createFilesController( - $folderMock, $trashManagerMock - ); - - $result = $filesController->getFileInfo(123); - assertSame($this->notFoundResponse, $result->getData()); - assertSame(404, $result->getStatus()); - } - public function testGetFileInfoFileExistingButNotReadable(): void { $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); $folderMock->method('getById')->willReturn([]); - - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById')->willReturn(null); - $mountCacheMock = $this->getMockBuilder('\OCP\Files\Config\IUserMountCache')->getMock(); $mountCacheMock->method('getMountsForFileId') ->willReturn( @@ -229,7 +156,7 @@ public function testGetFileInfoFileExistingButNotReadable(): void { ); $filesController = $this->createFilesController( - $folderMock, $trashManagerMock, $mountCacheMock + $folderMock, $mountCacheMock ); $result = $filesController->getFileInfo(759); @@ -258,38 +185,6 @@ public function testGetFileInfoFileExistingButCannotGetNameInContextOfOwner(): v assertSame(200, $result->getStatus()); } - public function testGetFilesInfoOneIdRequestedFileInTrash(): void { - $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); - $folderMock->method('getById')->willReturn([]); - - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById')->willReturn( - $this->getNodeMock('text/plain', 759, 'file', '/testUser/files_trashbin/files/welcome.txt.d1648724302') - ); - - $appManagerMock = $this->getMockBuilder('\OCP\App\IAppManager')->getMock(); - $appManagerMock->method('isEnabledForUser')->willReturn( - true - ); - - $mountCacheMock = $this->getSimpleMountCacheMock( - 'files_trashbin/files/welcome.txt.d1648724302' - ); - - $filesController = $this->getFilesControllerMock( - ['getDavPermissions'], $folderMock, $mountCacheMock, true, null, $trashManagerMock - ); - $filesController->method('getDavPermissions')->willReturn('RGDNVW'); - $result = $filesController->getFilesInfo([759]); - assertSame( - [ - 759 => $this->trashedWelcomeTxtResult, - ], - $result->getData() - ); - assertSame(200, $result->getStatus()); - } - public function testGetFilesInfoFourIdsRequestedOneExistsOneInTrashOneNotExisitingOneForbidden(): void { $folderMock = $this->getMockBuilder('\OCP\Files\Folder')->getMock(); $folderMock->method('getById') @@ -302,16 +197,6 @@ public function testGetFilesInfoFourIdsRequestedOneExistsOneInTrashOneNotExisiti [] ); - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById') - ->withConsecutive([$this->anything(), 759], [$this->anything(), 365], [$this->anything(), 956]) - ->willReturnOnConsecutiveCalls( - $this->getNodeMock('text/plain', 759, 'file', '/testUser/files_trashbin/files/welcome.txt.d1648724302'), - null, - null - ); - - $cachedMountFileInfoMock = $this->getMockBuilder( '\OCP\Files\Config\ICachedMountFileInfo' )->getMock(); @@ -319,8 +204,7 @@ public function testGetFilesInfoFourIdsRequestedOneExistsOneInTrashOneNotExisiti ->willReturnOnConsecutiveCalls( 'files/logo.png', 'files_trashbin/files/welcome.txt.d1648724302', - [], - [] + '/anotherUser/files/logo.png' ); $ownerMock = $this->getMockBuilder('\OCP\IUser')->getMock(); @@ -339,7 +223,7 @@ public function testGetFilesInfoFourIdsRequestedOneExistsOneInTrashOneNotExisiti ); $filesController = $this->getFilesControllerMock( - ['getDavPermissions'], $folderMock, $mountCacheMock, true, null, $trashManagerMock + ['getDavPermissions'], $folderMock, $mountCacheMock ); $filesController->method('getDavPermissions') ->willReturnOnConsecutiveCalls('RGDNVW', 'RGDNVW'); @@ -348,7 +232,7 @@ public function testGetFilesInfoFourIdsRequestedOneExistsOneInTrashOneNotExisiti assertSame( [ 123 => $this->logoPngResult, - 759 => $this->trashedWelcomeTxtResult, + 759 => $this->notFoundResponse, 365 => $this->notFoundResponse, 956 => $this->forbiddenResponse ], @@ -387,11 +271,11 @@ public function testGetFilesInfoThreeIdsRequestedOneFileExistsReturnsOneResult() $folderMock->method('getById') ->withConsecutive([123], [256], [365]) ->willReturnOnConsecutiveCalls( - [ - $this->getNodeMock('image/png', 123, 'file', '/testUser/files/logo.png') - ], - [], - [] + [ + $this->getNodeMock('image/png', 123, 'file', '/testUser/files/logo.png') + ], + [], + [] ); $cachedMountFileInfoMock = $this->getMockBuilder( @@ -582,7 +466,6 @@ public function testGetFilesInfoTwoIdsRequestedEachReturnsOneFolder(): void { 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVCK', @@ -599,7 +482,6 @@ public function testGetFilesInfoTwoIdsRequestedEachReturnsOneFolder(): void { 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVCK', @@ -684,7 +566,6 @@ public function testGetFilesInfoSendStringIds(): void { 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVCK', @@ -701,7 +582,6 @@ public function testGetFilesInfoSendStringIds(): void { 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVCK', @@ -861,7 +741,6 @@ public function testDavPermissions( "size" => 200245, "owner_name" => "Test User", "owner_id" => "3df8ff78-49cb-4d60-8d8b-171b29591fd3", - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => $davPermission, @@ -889,27 +768,6 @@ public function testDavPermissions( 'statuscode' => 403 ]; - /** - * @var array - */ - private array $trashedWelcomeTxtResult = [ - 'status' => 'OK', - 'statuscode' => 200, - "id" => 759, - "name" => 'welcome.txt.d1648724302', - "mtime" => 1640008813, - "ctime" => 1639906930, - "mimetype" => 'text/plain', - "size" => 200245, - "owner_name" => "Test User", - "owner_id" => "3df8ff78-49cb-4d60-8d8b-171b29591fd3", - "trashed" => true, - 'modifier_name' => null, - 'modifier_id' => null, - 'dav_permissions' => 'RGDNVW', - 'path' => 'files_trashbin/files/welcome.txt.d1648724302' - ]; - /** * @var array */ @@ -924,7 +782,6 @@ public function testDavPermissions( 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVW', @@ -945,7 +802,6 @@ public function testDavPermissions( 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVW', @@ -966,7 +822,6 @@ public function testDavPermissions( 'size' => 200245, 'owner_name' => 'Test User', 'owner_id' => '3df8ff78-49cb-4d60-8d8b-171b29591fd3', - 'trashed' => false, 'modifier_name' => null, 'modifier_id' => null, 'dav_permissions' => 'RGDNVW', @@ -975,15 +830,12 @@ public function testDavPermissions( /** * @param MockObject $folderMock - * @param MockObject|ITrashManager|null $trashManagerMock * @param MockObject|null $mountCacheMock mock for Files that exist but cannot be accessed by this user * @return FilesController */ private function createFilesController( MockObject $folderMock, - $trashManagerMock = null, - MockObject $mountCacheMock = null, - bool $isAppEnabled = true + MockObject $mountCacheMock = null ): FilesController { $storageMock = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock(); $storageMock->method('getUserFolder')->willReturn($folderMock); @@ -1003,10 +855,6 @@ private function createFilesController( 'OCP\Files\Config\IMountProviderCollection' )->getMock(); $mountProviderCollectionMock->method('getMountCache')->willReturn($mountCacheMock); - $appManagerMock = $this->getMockBuilder('\OCP\App\IAppManager')->getMock(); - $appManagerMock->method('isEnabledForUser')->willReturn( - $isAppEnabled - ); $controller = new FilesController( 'integration_openproject', @@ -1015,17 +863,11 @@ private function createFilesController( $userSessionMock, $mountProviderCollectionMock, $this->createMock(IManager::class), - $appManagerMock, $this->createMock(IDBConnection::class), $this->createMock(ILogger::class), $this->createMock(IUserManager::class), $this->createMock(DavUtil::class), ); - if ($trashManagerMock === null) { - $trashManagerMock = $this->getMockBuilder('\OCA\Files_Trashbin\Trash\ITrashManager')->getMock(); - $trashManagerMock->method('getTrashNodeById')->willReturn(null); - } - $controller->setTrashManager($trashManagerMock); return $controller; } @@ -1033,18 +875,14 @@ private function createFilesController( * @param array $onlyMethods * @param MockObject $folderMock * @param MockObject|null $mountCacheMock mock for Files that exist but cannot be accessed by this user - * @param bool $isAppEnabled * @param MockObject|null $davUtilsMock - * @param MockObject|ITrashManager|null $trashManagerMock * @return FilesController|MockObject */ public function getFilesControllerMock( array $onlyMethods, MockObject $folderMock, MockObject $mountCacheMock = null, - bool $isAppEnabled = true, - MockObject $davUtilsMock = null, - $trashManagerMock = null + MockObject $davUtilsMock = null ): FilesController { $storageMock = $this->getMockBuilder('\OCP\Files\IRootFolder')->getMock(); $storageMock->method('getUserFolder')->willReturn($folderMock); @@ -1064,31 +902,25 @@ public function getFilesControllerMock( } $mountProviderCollectionMock = $this->getMockBuilder( - 'OCP\Files\Config\IMountProviderCollection' + 'OCP\Files\Config\IMountProviderCollection' )->getMock(); $mountProviderCollectionMock->method('getMountCache')->willReturn($mountCacheMock); - $appManagerMock = $this->getMockBuilder('\OCP\App\IAppManager')->getMock(); - $appManagerMock->method('isEnabledForUser')->willReturn($isAppEnabled); $controller = $this->getMockBuilder(FilesController::class) - ->setConstructorArgs( - [ - 'integration_openproject', - $this->createMock(IRequest::class), - $storageMock, - $userSessionMock, - $mountProviderCollectionMock, - $this->createMock(IManager::class), - $appManagerMock, - $this->createMock(IDBConnection::class), - $this->createMock(ILogger::class), - $this->createMock(IUserManager::class), - $davUtilsMock - ]) - ->onlyMethods($onlyMethods) - ->getMock(); - if ($trashManagerMock) { - $controller->setTrashManager($trashManagerMock); - } + ->setConstructorArgs( + [ + 'integration_openproject', + $this->createMock(IRequest::class), + $storageMock, + $userSessionMock, + $mountProviderCollectionMock, + $this->createMock(IManager::class), + $this->createMock(IDBConnection::class), + $this->createMock(ILogger::class), + $this->createMock(IUserManager::class), + $davUtilsMock + ]) + ->onlyMethods($onlyMethods) + ->getMock(); return $controller; }