From a1064f5c61217418a861b55471bc06971d39bf6f 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 746832e9280cd..63dfffa7eb62d 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -12,6 +12,7 @@ use OCP\Constants; use OCP\Files\Cache\ICacheEntry; use OCP\Files\IMimeTypeDetector; +use OCP\Files\Mount\IMountPoint; use OCP\Server; class Helper { @@ -26,9 +27,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)) { @@ -39,50 +37,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 !== '') { @@ -91,6 +52,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 357d096d8cc46fbceee5e05472eec07d738c8b9f 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 63dfffa7eb62d..159967055d8b2 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -52,6 +52,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 dd89583d9a1b3..261767b9d56e6 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -56,35 +56,29 @@ 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; } 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); } public function childExists($name): bool { - try { - $this->getChild($name); - return true; - } catch (NotFound $e) { - return false; - } + return $this->trashManager->getTrashRootItem($this->user, $name) !== null; } public function getLastModified(): int { diff --git a/apps/files_trashbin/lib/Trash/ITrashManager.php b/apps/files_trashbin/lib/Trash/ITrashManager.php index 743ea01358a1d..4aab456787fdf 100644 --- a/apps/files_trashbin/lib/Trash/ITrashManager.php +++ b/apps/files_trashbin/lib/Trash/ITrashManager.php @@ -27,6 +27,15 @@ public function registerBackend(string $storageType, ITrashBackend $backend); */ 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 204defde35c72..1188356f440ae 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -63,6 +63,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]; + } + public function listTrashFolder(ITrashItem $folder): array { $user = $folder->getUser(); $entries = Helper::getTrashFiles($folder->getTrashPath(), $user->getUID()); diff --git a/apps/files_trashbin/lib/Trash/TrashManager.php b/apps/files_trashbin/lib/Trash/TrashManager.php index 521a576c00a7b..04cac30f8f4e9 100644 --- a/apps/files_trashbin/lib/Trash/TrashManager.php +++ b/apps/files_trashbin/lib/Trash/TrashManager.php @@ -36,6 +36,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 610914b411d5d638820ed6089cd1d39924980c6d 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 261767b9d56e6..1f3c506c664af 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -14,15 +14,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, ) { } @@ -56,12 +58,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); } public function getChild($name): ITrash { @@ -71,10 +68,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); } public function childExists($name): bool { @@ -84,4 +78,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 1e4637e875dc395c1994b730c61c4ef7c61ce2a5 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 159967055d8b2..e4fb4c46c2bf6 100644 --- a/apps/files_trashbin/lib/Helper.php +++ b/apps/files_trashbin/lib/Helper.php @@ -118,21 +118,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 86cec9866bacdf9f6d90ce86bf30d388fed27901 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 | 8 --- 3 files changed, 22 insertions(+), 37 deletions(-) diff --git a/apps/files_trashbin/lib/Sabre/TrashRoot.php b/apps/files_trashbin/lib/Sabre/TrashRoot.php index 1f3c506c664af..f44eaa4340680 100644 --- a/apps/files_trashbin/lib/Sabre/TrashRoot.php +++ b/apps/files_trashbin/lib/Sabre/TrashRoot.php @@ -83,7 +83,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 1188356f440ae..2117596eda802 100644 --- a/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php +++ b/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php @@ -29,38 +29,31 @@ 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, + ); } 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 { @@ -68,13 +61,13 @@ public function getTrashRootItem(IUser $user, string $name): ?ITrashItem { if ($entry === null) { return null; } - return $this->mapTrashItems([$entry], $user)[0]; + return $this->mapTrashItem($entry, $user); } 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); } public function restoreItem(ITrashItem $item) { diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index b2ce8989bd72a..b69ada09a7c7f 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1828,14 +1828,6 @@ - - - - - - - - From 43438c1f7847ba9d4b19da00ea594faa06798508 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 15 Jul 2026 15:30:23 +0200 Subject: [PATCH 6/6] fix: Adjust API @since version Signed-off-by: Carl Schwan --- apps/files_trashbin/lib/Trash/ITrashManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_trashbin/lib/Trash/ITrashManager.php b/apps/files_trashbin/lib/Trash/ITrashManager.php index 4aab456787fdf..5547861a06db1 100644 --- a/apps/files_trashbin/lib/Trash/ITrashManager.php +++ b/apps/files_trashbin/lib/Trash/ITrashManager.php @@ -32,7 +32,7 @@ public function listTrashRoot(IUser $user): array; * * @param IUser $user * @return ?ITrashItem - * @since 35.0.0 + * @since 32.0.13 */ public function getTrashRootItem(IUser $user, string $name): ?ITrashItem;