From 7d77d0d5f7a92bd8ff98b0cddffec9f2a19d1a79 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Jul 2026 10:18:30 +0200 Subject: [PATCH 1/6] refactor: Extract buildFileInfo in a seperate method Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Helper.php | 92 ++++++++++++++++-------------- 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/apps/files_trashbin/lib/Helper.php b/apps/files_trashbin/lib/Helper.php index fc7e97ec8f80d..177b77e49aab6 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -13,6 +13,7 @@ use OCP\Constants; use OCP\Files\Cache\ICacheEntry; use OCP\Files\IMimeTypeDetector; +use OCP\Files\Mount\IMountPoint; use OCP\Server; class Helper { @@ -27,9 +28,6 @@ class Helper { * @return \OCP\Files\FileInfo[] */ public static function getTrashFiles($dir, $user, $sortAttribute = '', $sortDescending = false) { - $result = []; - $timestamp = null; - $view = new View('/' . $user . '/files_trashbin/files'); if (ltrim($dir, '/') !== '' && !$view->is_dir($dir)) { @@ -40,50 +38,13 @@ public static function getTrashFiles($dir, $user, $sortAttribute = '', $sortDesc $storage = $mount->getStorage(); $absoluteDir = $view->getAbsolutePath($dir); $internalPath = $mount->getInternalPath($absoluteDir); - $extraData = Trashbin::getExtraData($user); + + $result = []; + $timestamp = null; $dirContent = $storage->getCache()->getFolderContents($mount->getInternalPath($view->getAbsolutePath($dir))); foreach ($dirContent as $entry) { - $entryName = $entry->getName(); - $name = $entryName; - if ($dir === '' || $dir === '/') { - $pathparts = pathinfo($entryName); - $timestamp = substr($pathparts['extension'], 1); - $name = $pathparts['filename']; - } elseif ($timestamp === null) { - // for subfolders we need to calculate the timestamp only once - $parts = explode('/', ltrim($dir, '/')); - $timestamp = substr(pathinfo($parts[0], PATHINFO_EXTENSION), 1); - } - $originalPath = ''; - $originalName = substr($entryName, 0, -strlen($timestamp) - 2); - if (isset($extraData[$originalName][$timestamp]['location'])) { - $originalPath = $extraData[$originalName][$timestamp]['location']; - if (substr($originalPath, -1) === '/') { - $originalPath = substr($originalPath, 0, -1); - } - } - $type = $entry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE ? 'dir' : 'file'; - $i = [ - 'name' => $name, - 'mtime' => $timestamp, - 'mimetype' => $type === 'dir' ? 'httpd/unix-directory' : Server::get(IMimeTypeDetector::class)->detectPath($name), - 'type' => $type, - 'directory' => ($dir === '/') ? '' : $dir, - 'size' => $entry->getSize(), - 'etag' => '', - 'permissions' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, - 'fileid' => $entry->getId(), - ]; - if ($originalPath) { - if ($originalPath !== '.') { - $i['extraData'] = $originalPath . '/' . $originalName; - } else { - $i['extraData'] = $originalName; - } - } - $i['deletedBy'] = $extraData[$originalName][$timestamp]['deletedBy'] ?? null; - $result[] = new FileInfo($absoluteDir . '/' . $i['name'], $storage, $internalPath . '/' . $i['name'], $i, $mount); + $result[] = self::buildFileInfo($entry, $dir, $timestamp, $extraData, $storage, $absoluteDir, $internalPath, $mount); } if ($sortAttribute !== '') { @@ -92,6 +53,49 @@ public static function getTrashFiles($dir, $user, $sortAttribute = '', $sortDesc return $result; } + private static function buildFileInfo(ICacheEntry $entry, string $dir, ?string &$timestamp, array $extraData, $storage, string $absoluteDir, string $internalPath, IMountPoint $mount): FileInfo { + $entryName = $entry->getName(); + $name = $entryName; + if ($dir === '' || $dir === '/') { + $pathparts = pathinfo($entryName); + $timestamp = substr($pathparts['extension'], 1); + $name = $pathparts['filename']; + } elseif ($timestamp === null) { + // for subfolders we need to calculate the timestamp only once + $parts = explode('/', ltrim($dir, '/')); + $timestamp = substr(pathinfo($parts[0], PATHINFO_EXTENSION), 1); + } + $originalPath = ''; + $originalName = substr($entryName, 0, -strlen($timestamp) - 2); + if (isset($extraData[$originalName][$timestamp]['location'])) { + $originalPath = $extraData[$originalName][$timestamp]['location']; + if (substr($originalPath, -1) === '/') { + $originalPath = substr($originalPath, 0, -1); + } + } + $type = $entry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE ? 'dir' : 'file'; + $i = [ + 'name' => $name, + 'mtime' => $timestamp, + 'mimetype' => $type === 'dir' ? 'httpd/unix-directory' : Server::get(IMimeTypeDetector::class)->detectPath($name), + 'type' => $type, + 'directory' => ($dir === '/') ? '' : $dir, + 'size' => $entry->getSize(), + 'etag' => '', + 'permissions' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, + 'fileid' => $entry->getId(), + ]; + if ($originalPath) { + if ($originalPath !== '.') { + $i['extraData'] = $originalPath . '/' . $originalName; + } else { + $i['extraData'] = $originalName; + } + } + $i['deletedBy'] = $extraData[$originalName][$timestamp]['deletedBy'] ?? null; + return new FileInfo($absoluteDir . '/' . $i['name'], $storage, $internalPath . '/' . $i['name'], $i, $mount); + } + /** * Format file infos for JSON * From 3d63b0fffabb133a6a79b9da9915253cedf7ebb5 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 8 Jul 2026 11:35:10 +0200 Subject: [PATCH 2/6] perf: Don't fetch all the trash items when deleting a single item Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Helper.php | 24 +++++++++++++++++++ apps/files_trashbin/lib/Sabre/TrashRoot.php | 24 +++++++------------ .../lib/Trash/ITrashManager.php | 9 +++++++ .../lib/Trash/LegacyTrashBackend.php | 8 +++++++ .../files_trashbin/lib/Trash/TrashManager.php | 20 ++++++++++++++++ 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/apps/files_trashbin/lib/Helper.php b/apps/files_trashbin/lib/Helper.php index 177b77e49aab6..2a4ebb5ad6c12 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -53,6 +53,30 @@ public static function getTrashFiles($dir, $user, $sortAttribute = '', $sortDesc return $result; } + public static function getTrashFile(string $dir, string $user, string $name): ?FileInfo { + $timestamp = null; + + $view = new View('/' . $user . '/files_trashbin/files'); + + if (ltrim($dir, '/') !== '' && !$view->is_dir($dir)) { + throw new \Exception('Directory does not exists'); + } + + $mount = $view->getMount($dir); + $storage = $mount->getStorage(); + $absoluteDir = $view->getAbsolutePath($dir); + $internalPath = $mount->getInternalPath($absoluteDir); + + $extraData = Trashbin::getExtraData($user); + $entry = $storage->getCache()->get($mount->getInternalPath($view->getAbsolutePath($dir . '/' . $name))); + if ($entry === false) { + return null; + } + + $timestamp = null; + return self::buildFileInfo($entry, $dir, $timestamp, $extraData, $storage, $absoluteDir, $internalPath, $mount); + } + private static function buildFileInfo(ICacheEntry $entry, string $dir, ?string &$timestamp, array $extraData, $storage, string $absoluteDir, string $internalPath, IMountPoint $mount): FileInfo { $entryName = $entry->getName(); $name = $entryName; diff --git a/apps/files_trashbin/lib/Sabre/TrashRoot.php b/apps/files_trashbin/lib/Sabre/TrashRoot.php index 9c730c69d8770..7da8573ab7dd2 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -63,37 +63,31 @@ public function createDirectory($name) { public function getChildren(): array { $entries = $this->trashManager->listTrashRoot($this->user); - $children = array_map(function (ITrashItem $entry) { + return array_map(function (ITrashItem $entry): TrashFile|TrashFolder { if ($entry->getType() === FileInfo::TYPE_FOLDER) { return new TrashFolder($this->trashManager, $entry); } return new TrashFile($this->trashManager, $entry); }, $entries); - - return $children; } #[\Override] public function getChild($name): ITrash { - $entries = $this->getChildren(); + $entry = $this->trashManager->getTrashRootItem($this->user, $name); - foreach ($entries as $entry) { - if ($entry->getName() === $name) { - return $entry; - } + if ($entry === null) { + throw new NotFound(); } - throw new NotFound(); + if ($entry->getType() === FileInfo::TYPE_FOLDER) { + return new TrashFolder($this->trashManager, $entry); + } + return new TrashFile($this->trashManager, $entry); } #[\Override] public function childExists($name): bool { - try { - $this->getChild($name); - return true; - } catch (NotFound $e) { - return false; - } + return $this->trashManager->getTrashRootItem($this->user, $name) !== null; } #[\Override] diff --git a/apps/files_trashbin/lib/Trash/ITrashManager.php b/apps/files_trashbin/lib/Trash/ITrashManager.php index 25a07d66c8b8c..f5188dc19de71 100644 --- a/apps/files_trashbin/lib/Trash/ITrashManager.php +++ b/apps/files_trashbin/lib/Trash/ITrashManager.php @@ -31,6 +31,15 @@ public function registerBackend(string $storageType, ITrashBackend $backend); #[\Override] public function listTrashRoot(IUser $user): array; + /** + * Get a specific item in the root of the trashbin + * + * @param IUser $user + * @return ?ITrashItem + * @since 35.0.0 + */ + public function getTrashRootItem(IUser $user, string $name): ?ITrashItem; + /** * Temporally prevent files from being moved to the trash * diff --git a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php index 54ffd6f4cabb9..8bd64d83011a6 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -65,6 +65,14 @@ public function listTrashRoot(IUser $user): array { return $this->mapTrashItems($entries, $user); } + public function getTrashRootItem(IUser $user, string $name): ?ITrashItem { + $entry = Helper::getTrashFile('/', $user->getUID(), $name); + if ($entry === null) { + return null; + } + return $this->mapTrashItems([$entry], $user)[0]; + } + #[\Override] public function listTrashFolder(ITrashItem $folder): array { $user = $folder->getUser(); diff --git a/apps/files_trashbin/lib/Trash/TrashManager.php b/apps/files_trashbin/lib/Trash/TrashManager.php index 3706a6a2e8033..cf0f34cff6d69 100644 --- a/apps/files_trashbin/lib/Trash/TrashManager.php +++ b/apps/files_trashbin/lib/Trash/TrashManager.php @@ -39,6 +39,26 @@ public function listTrashRoot(IUser $user): array { return $items; } + #[\Override] + public function getTrashRootItem(IUser $user, string $name): ?ITrashItem { + foreach ($this->getBackends() as $backend) { + if (method_exists($backend, 'getTrashRootItem')) { + $item = $backend->getTrashRootItem($user, $name); + if ($item !== null) { + return $item; + } + } else { + $items = $backend->listTrashRoot($user); + foreach ($items as $item) { + if ($item->getName() === $name) { + return $item; + } + } + } + } + return null; + } + private function getBackendForItem(ITrashItem $item) { return $item->getTrashBackend(); } From f75b6fc5caa1686f2ee7cb33f8dba731e9193e1e Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Jul 2026 10:54:09 +0200 Subject: [PATCH 3/6] refactor: Extract and improve ITrashItem mapping to INode Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Sabre/TrashRoot.php | 26 +++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/apps/files_trashbin/lib/Sabre/TrashRoot.php b/apps/files_trashbin/lib/Sabre/TrashRoot.php index 7da8573ab7dd2..30e034eb53790 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -15,15 +15,17 @@ use OCA\Files_Trashbin\Trashbin; use OCP\Files\FileInfo; use OCP\IUser; +use RuntimeException; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\ICollection; +use Sabre\DAV\INode; class TrashRoot implements ICollection { public function __construct( - private IUser $user, - private ITrashManager $trashManager, + private readonly IUser $user, + private readonly ITrashManager $trashManager, ) { } @@ -63,12 +65,7 @@ public function createDirectory($name) { public function getChildren(): array { $entries = $this->trashManager->listTrashRoot($this->user); - return array_map(function (ITrashItem $entry): TrashFile|TrashFolder { - if ($entry->getType() === FileInfo::TYPE_FOLDER) { - return new TrashFolder($this->trashManager, $entry); - } - return new TrashFile($this->trashManager, $entry); - }, $entries); + return array_map($this->trashItemToTrashNode(...), $entries); } #[\Override] @@ -79,10 +76,7 @@ public function getChild($name): ITrash { throw new NotFound(); } - if ($entry->getType() === FileInfo::TYPE_FOLDER) { - return new TrashFolder($this->trashManager, $entry); - } - return new TrashFile($this->trashManager, $entry); + return $this->trashItemToTrashNode($entry); } #[\Override] @@ -94,4 +88,12 @@ public function childExists($name): bool { public function getLastModified(): int { return 0; } + + private function trashItemToTrashNode(ITrashItem $entry): ITrash&INode { + return match ($entry->getType()) { + FileInfo::TYPE_FOLDER => new TrashFolder($this->trashManager, $entry), + FileInfo::TYPE_FILE => new TrashFile($this->trashManager, $entry), + default => throw new RuntimeException("Invalid FileInfo object"), + }; + } } From 8200bddc05f9ab7abfd00e08c792b6a305c3d6a6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Jul 2026 11:02:03 +0200 Subject: [PATCH 4/6] refactor: Remove some dead code from the trashbin Helper Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Helper.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/apps/files_trashbin/lib/Helper.php b/apps/files_trashbin/lib/Helper.php index 2a4ebb5ad6c12..cdfa847633b14 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -119,21 +119,4 @@ private static function buildFileInfo(ICacheEntry $entry, string $dir, ?string & $i['deletedBy'] = $extraData[$originalName][$timestamp]['deletedBy'] ?? null; return new FileInfo($absoluteDir . '/' . $i['name'], $storage, $internalPath . '/' . $i['name'], $i, $mount); } - - /** - * Format file infos for JSON - * - * @param \OCP\Files\FileInfo[] $fileInfos file infos - */ - public static function formatFileInfos($fileInfos) { - $files = []; - foreach ($fileInfos as $i) { - $entry = \OCA\Files\Helper::formatFileInfo($i); - $entry['id'] = $i->getId(); - $entry['etag'] = $entry['mtime']; // add fake etag, it is only needed to identify the preview image - $entry['permissions'] = Constants::PERMISSION_READ; - $files[] = $entry; - } - return $files; - } } From 47fe579498cef718abf404631e8ec0b733668e79 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Jul 2026 11:07:02 +0200 Subject: [PATCH 5/6] refactor(trashbin): Replace mapTrashItems by mapTrashItem Only take one argument and instead call this with array map Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Sabre/TrashRoot.php | 2 +- .../lib/Trash/LegacyTrashBackend.php | 49 ++++++++----------- build/psalm-baseline.xml | 14 +++--- 3 files changed, 28 insertions(+), 37 deletions(-) diff --git a/apps/files_trashbin/lib/Sabre/TrashRoot.php b/apps/files_trashbin/lib/Sabre/TrashRoot.php index 30e034eb53790..0d124c3d6eb1a 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -93,7 +93,7 @@ private function trashItemToTrashNode(ITrashItem $entry): ITrash&INode { return match ($entry->getType()) { FileInfo::TYPE_FOLDER => new TrashFolder($this->trashManager, $entry), FileInfo::TYPE_FILE => new TrashFile($this->trashManager, $entry), - default => throw new RuntimeException("Invalid FileInfo object"), + default => throw new RuntimeException('Invalid FileInfo object'), }; } } diff --git a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php index 8bd64d83011a6..5dbba8d60e191 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -30,39 +30,32 @@ public function __construct( ) { } - /** - * @param array $items - * @param IUser $user - * @param ITrashItem $parent - * @return ITrashItem[] - */ - private function mapTrashItems(array $items, IUser $user, ?ITrashItem $parent = null): array { + private function mapTrashItem(FileInfo $file, IUser $user, ?ITrashItem $parent = null): ITrashItem { $parentTrashPath = ($parent instanceof ITrashItem) ? $parent->getTrashPath() : ''; $isRoot = $parent === null; - return array_map(function (FileInfo $file) use ($parent, $parentTrashPath, $isRoot, $user) { - $originalLocation = $isRoot ? $file['extraData'] : $parent->getOriginalLocation() . '/' . $file->getName(); - if (!$originalLocation) { - $originalLocation = $file->getName(); - } - /** @psalm-suppress UndefinedInterfaceMethod */ - $deletedBy = $this->userManager->get($file['deletedBy']) ?? $parent?->getDeletedBy(); - $trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime()); - return new TrashItem( - $this, - $originalLocation, - $file->getMTime(), - $parentTrashPath . '/' . ($isRoot ? $trashFilename : $file->getName()), - $file, - $user, - $deletedBy, - ); - }, $items); + + $originalLocation = $isRoot ? $file['extraData'] : $parent->getOriginalLocation() . '/' . $file->getName(); + if (!$originalLocation) { + $originalLocation = $file->getName(); + } + /** @psalm-suppress UndefinedInterfaceMethod */ + $deletedBy = $this->userManager->get($file['deletedBy']) ?? $parent?->getDeletedBy(); + $trashFilename = Trashbin::getTrashFilename($file->getName(), $file->getMtime()); + return new TrashItem( + $this, + $originalLocation, + $file->getMTime(), + $parentTrashPath . '/' . ($isRoot ? $trashFilename : $file->getName()), + $file, + $user, + $deletedBy, + ); } #[\Override] public function listTrashRoot(IUser $user): array { $entries = Helper::getTrashFiles('/', $user->getUID()); - return $this->mapTrashItems($entries, $user); + return array_map(fn (FileInfo $fileInfo): ITrashItem => $this->mapTrashItem($fileInfo, $user), $entries); } public function getTrashRootItem(IUser $user, string $name): ?ITrashItem { @@ -70,14 +63,14 @@ public function getTrashRootItem(IUser $user, string $name): ?ITrashItem { if ($entry === null) { return null; } - return $this->mapTrashItems([$entry], $user)[0]; + return $this->mapTrashItem($entry, $user); } #[\Override] public function listTrashFolder(ITrashItem $folder): array { $user = $folder->getUser(); $entries = Helper::getTrashFiles($folder->getTrashPath(), $user->getUID()); - return $this->mapTrashItems($entries, $user, $folder); + return array_map(fn (FileInfo $fileInfo): ITrashItem => $this->mapTrashItem($fileInfo, $user, $folder), $entries); } #[\Override] diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 058019a00adeb..02983e913bad4 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1877,12 +1877,18 @@ + + + + + + @@ -1915,14 +1921,6 @@ - - - - - - - - From b5dd8999ea3e73b8b3f06bd6e87bf7361484ec9c Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 9 Jul 2026 11:09:05 +0200 Subject: [PATCH 6/6] refactor: Micro cleanup in LegacyTrashBackend Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Trash/LegacyTrashBackend.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php index 5dbba8d60e191..11c4bc18321c3 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -15,18 +15,19 @@ use OCP\Files\FileInfo; use OCP\Files\Folder; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; use OCP\IUser; use OCP\IUserManager; class LegacyTrashBackend implements ITrashBackend { - /** @var array */ - private $deletedFiles = []; + /** @var array */ + private array $deletedFiles = []; public function __construct( - private IRootFolder $rootFolder, - private IUserManager $userManager, + private readonly IRootFolder $rootFolder, + private readonly IUserManager $userManager, ) { } @@ -113,7 +114,7 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool { } #[\Override] - public function getTrashNodeById(IUser $user, int $fileId) { + public function getTrashNodeById(IUser $user, int $fileId): ?Node { try { $userFolder = $this->rootFolder->getUserFolder($user->getUID()); $trash = $userFolder->getParent()->get('files_trashbin/files');