From a8d69ffed1892f23629f291bbee4dcc3b56c3669 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 4 Sep 2025 14:07:14 +0200 Subject: [PATCH] refactor: Commands and background jobs for the trashbin - Use modern node and SetupManager API - Avoid passing the user by id and instead use IUser Signed-off-by: Carl Schwan Signed-off-by: Carl Schwan --- .../lib/BackgroundJob/ExpireTrash.php | 52 ++++++------ apps/files_trashbin/lib/Command/CleanUp.php | 82 +++++++++++-------- apps/files_trashbin/lib/Command/Expire.php | 36 +++++--- .../lib/Command/ExpireTrash.php | 77 ++++++++--------- .../lib/Command/RestoreAllFiles.php | 15 ++-- apps/files_trashbin/lib/Command/Size.php | 19 +++-- .../lib/Listener/EventListener.php | 22 ++++- apps/files_trashbin/lib/Trashbin.php | 46 ++++------- .../tests/BackgroundJob/ExpireTrashTest.php | 5 ++ .../tests/Command/CleanUpTest.php | 42 +++++++--- .../tests/Command/ExpireTrashTest.php | 11 +-- build/psalm-baseline.xml | 55 ------------- 12 files changed, 233 insertions(+), 229 deletions(-) diff --git a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php index 4d890f977b9..7305c24cc57 100644 --- a/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php +++ b/apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php @@ -7,18 +7,20 @@ */ namespace OCA\Files_Trashbin\BackgroundJob; -use OC\Files\View; use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\Expiration; use OCA\Files_Trashbin\Trashbin; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\Files\ISetupManager; use OCP\IAppConfig; use OCP\IUser; use OCP\IUserManager; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; +use Override; use Psr\Log\LoggerInterface; class ExpireTrash extends TimedJob { @@ -28,19 +30,21 @@ class ExpireTrash extends TimedJob { private const USER_BATCH_SIZE = 10; public function __construct( - private IAppConfig $appConfig, - private IUserManager $userManager, - private Expiration $expiration, - private LoggerInterface $logger, - private ISetupManager $setupManager, - private ILockingProvider $lockingProvider, + private readonly IAppConfig $appConfig, + private readonly IUserManager $userManager, + private readonly Expiration $expiration, + private readonly LoggerInterface $logger, + private readonly ISetupManager $setupManager, + private readonly ILockingProvider $lockingProvider, + private readonly IRootFolder $rootFolder, ITimeFactory $time, ) { parent::__construct($time); $this->setInterval(self::THIRTY_MINUTES); } - protected function run($argument) { + #[Override] + protected function run($argument): void { $backgroundJob = $this->appConfig->getValueBool(Application::APP_ID, self::TOGGLE_CONFIG_KEY_NAME, true); if (!$backgroundJob) { return; @@ -64,9 +68,8 @@ class ExpireTrash extends TimedJob { $count++; try { - if ($this->setupFS($user)) { - Trashbin::expire($uid); - } + $folder = $this->getTrashRoot($user); + Trashbin::expire($folder, $user); } catch (\Throwable $e) { $this->logger->error('Error while expiring trashbin for user ' . $uid, ['exception' => $e]); } finally { @@ -82,23 +85,19 @@ class ExpireTrash extends TimedJob { } } - /** - * Act on behalf on trash item owner - */ - protected function setupFS(IUser $user): bool { + private function getTrashRoot(IUser $user): Folder { + $this->setupManager->tearDown(); $this->setupManager->setupForUser($user); - // Check if this user has a trashbin directory - $view = new View('/' . $user->getUID()); - if (!$view->is_dir('/files_trashbin/files')) { - return false; + $folder = $this->rootFolder->getUserFolder($user->getUID())->getParent()->get('files_trashbin'); + if (!$folder instanceof Folder) { + throw new \LogicException("Didn't expect files_trashbin to be a file instead of a folder"); } - - return true; + return $folder; } private function getNextOffset(): int { - return $this->runMutexOperation(function () { + return $this->runMutexOperation(function (): int { $this->appConfig->clearCache(); $offset = $this->appConfig->getValueInt(Application::APP_ID, self::OFFSET_CONFIG_KEY_NAME, 0); @@ -109,13 +108,18 @@ class ExpireTrash extends TimedJob { } - private function resetOffset() { + private function resetOffset(): void { $this->runMutexOperation(function (): void { $this->appConfig->setValueInt(Application::APP_ID, self::OFFSET_CONFIG_KEY_NAME, 0); }); } - private function runMutexOperation($operation): mixed { + /** + * @template T + * @param callable(): T $operation + * @return T + */ + private function runMutexOperation(callable $operation): mixed { $acquired = false; while ($acquired === false) { diff --git a/apps/files_trashbin/lib/Command/CleanUp.php b/apps/files_trashbin/lib/Command/CleanUp.php index 88282e80ce3..4eaafd274a9 100644 --- a/apps/files_trashbin/lib/Command/CleanUp.php +++ b/apps/files_trashbin/lib/Command/CleanUp.php @@ -7,29 +7,36 @@ */ namespace OCA\Files_Trashbin\Command; +use OC\Core\Command\Base; +use OC\Files\SetupManager; +use OC\User\LazyUser; use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; use OCP\IDBConnection; +use OCP\IUser; use OCP\IUserBackend; use OCP\IUserManager; use OCP\Util; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidOptionException; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -class CleanUp extends Command { +class CleanUp extends Base { public function __construct( protected IRootFolder $rootFolder, protected IUserManager $userManager, protected IDBConnection $dbConnection, + protected SetupManager $setupManager, ) { parent::__construct(); } - protected function configure() { + protected function configure(): void { + parent::configure(); $this ->setName('trashbin:cleanup') ->setDescription('Remove deleted files') @@ -47,17 +54,18 @@ class CleanUp extends Command { } protected function execute(InputInterface $input, OutputInterface $output): int { - $users = $input->getArgument('user_id'); + $userIds = $input->getArgument('user_id'); $verbose = $input->getOption('verbose'); - if (!empty($users) && $input->getOption('all-users')) { + if (!empty($userIds) && $input->getOption('all-users')) { throw new InvalidOptionException('Either specify a user_id or --all-users'); - } elseif (!empty($users)) { - foreach ($users as $user) { - if ($this->userManager->userExists($user)) { - $output->writeln("Remove deleted files of $user"); + } elseif (!empty($userIds)) { + foreach ($userIds as $userId) { + $user = $this->userManager->get($userId); + if ($user) { + $output->writeln("Remove deleted files of $userId"); $this->removeDeletedFiles($user, $output, $verbose); } else { - $output->writeln("Unknown user $user"); + $output->writeln("Unknown user $userId"); return 1; } } @@ -72,13 +80,14 @@ class CleanUp extends Command { $limit = 500; $offset = 0; do { - $users = $backend->getUsers('', $limit, $offset); - foreach ($users as $user) { - $output->writeln(" $user"); + $userIds = $backend->getUsers('', $limit, $offset); + foreach ($userIds as $userId) { + $output->writeln(" $userId"); + $user = new LazyUser($userId, $this->userManager, null, $backend); $this->removeDeletedFiles($user, $output, $verbose); } $offset += $limit; - } while (count($users) >= $limit); + } while (count($userIds) >= $limit); } } else { throw new InvalidOptionException('Either specify a user_id or --all-users'); @@ -87,32 +96,33 @@ class CleanUp extends Command { } /** - * remove deleted files for the given user + * Remove deleted files for the given user. */ - protected function removeDeletedFiles(string $uid, OutputInterface $output, bool $verbose): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); - $path = '/' . $uid . '/files_trashbin'; - if ($this->rootFolder->nodeExists($path)) { + protected function removeDeletedFiles(IUser $user, OutputInterface $output, bool $verbose): void { + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); + $path = '/' . $user->getUID() . '/files_trashbin'; + try { $node = $this->rootFolder->get($path); - + } catch (NotFoundException|NotPermittedException) { if ($verbose) { - $output->writeln('Deleting ' . Util::humanFileSize($node->getSize()) . " in trash for $uid."); - } - $node->delete(); - if ($this->rootFolder->nodeExists($path)) { - $output->writeln('Trash folder sill exists after attempting to delete it'); - return; - } - $query = $this->dbConnection->getQueryBuilder(); - $query->delete('files_trash') - ->where($query->expr()->eq('user', $query->createParameter('uid'))) - ->setParameter('uid', $uid); - $query->executeStatement(); - } else { - if ($verbose) { - $output->writeln("No trash found for $uid"); + $output->writeln("No trash found for {$user->getUID()}"); } + return; } + + if ($verbose) { + $output->writeln('Deleting ' . Util::humanFileSize($node->getSize()) . " in trash for {$user->getUID()}."); + } + $node->delete(); + if ($this->rootFolder->nodeExists($path)) { + $output->writeln('Trash folder sill exists after attempting to delete it'); + return; + } + $query = $this->dbConnection->getQueryBuilder(); + $query->delete('files_trash') + ->where($query->expr()->eq('user', $query->createParameter('uid'))) + ->setParameter('uid', $user->getUID()); + $query->executeStatement(); } } diff --git a/apps/files_trashbin/lib/Command/Expire.php b/apps/files_trashbin/lib/Command/Expire.php index 73a42cd4749..233be4ee5f9 100644 --- a/apps/files_trashbin/lib/Command/Expire.php +++ b/apps/files_trashbin/lib/Command/Expire.php @@ -8,32 +8,48 @@ namespace OCA\Files_Trashbin\Command; use OC\Command\FileAccess; +use OC\Files\SetupManager; use OCA\Files_Trashbin\Trashbin; use OCP\Command\ICommand; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\IUserManager; use OCP\Server; +use Override; +use Psr\Log\LoggerInterface; class Expire implements ICommand { use FileAccess; - /** - * @param string $user - */ public function __construct( - private $user, + private readonly string $userId, ) { } - public function handle() { + #[Override] + public function handle(): void { + // can't use DI because Expire needs to be serializable $userManager = Server::get(IUserManager::class); - if (!$userManager->userExists($this->user)) { + $user = $userManager->get($this->userId); + if (!$user) { // User has been deleted already return; } - \OC_Util::tearDownFS(); - \OC_Util::setupFS($this->user); - Trashbin::expire($this->user); - \OC_Util::tearDownFS(); + try { + $setupManager = Server::get(SetupManager::class); + $setupManager->tearDown(); + $setupManager->setupForUser($user); + + $trashRoot = Server::get(IRootFolder::class)->getUserFolder($user->getUID())->getParent()->get('files_trashbin'); + if (!$trashRoot instanceof Folder) { + throw new \LogicException("Didn't expect files_trashbin to be a file instead of a folder"); + } + Trashbin::expire($trashRoot, $user); + } catch (\Exception $e) { + Server::get(LoggerInterface::class)->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]); + } finally { + $setupManager->tearDown(); + } } } diff --git a/apps/files_trashbin/lib/Command/ExpireTrash.php b/apps/files_trashbin/lib/Command/ExpireTrash.php index 422d8379984..fdc8e058cbf 100644 --- a/apps/files_trashbin/lib/Command/ExpireTrash.php +++ b/apps/files_trashbin/lib/Command/ExpireTrash.php @@ -7,33 +7,32 @@ */ namespace OCA\Files_Trashbin\Command; -use OC\Files\View; +use OC\Core\Command\Base; +use OC\Files\SetupManager; use OCA\Files_Trashbin\Expiration; use OCA\Files_Trashbin\Trashbin; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\IUser; use OCP\IUserManager; -use Psr\Log\LoggerInterface; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -class ExpireTrash extends Command { +class ExpireTrash extends Base { - /** - * @param IUserManager|null $userManager - * @param Expiration|null $expiration - */ public function __construct( - private LoggerInterface $logger, - private ?IUserManager $userManager = null, - private ?Expiration $expiration = null, + private readonly ?IUserManager $userManager, + private readonly ?Expiration $expiration, + private readonly SetupManager $setupManager, + private readonly IRootFolder $rootFolder, ) { parent::__construct(); } - protected function configure() { + protected function configure(): void { + parent::configure(); $this ->setName('trashbin:expire') ->setDescription('Expires the users trashbin') @@ -52,15 +51,17 @@ class ExpireTrash extends Command { return 1; } - $users = $input->getArgument('user_id'); - if (!empty($users)) { - foreach ($users as $user) { - if ($this->userManager->userExists($user)) { - $output->writeln("Remove deleted files of $user"); - $userObject = $this->userManager->get($user); - $this->expireTrashForUser($userObject); + $userIds = $input->getArgument('user_id'); + if (!empty($userIds)) { + foreach ($userIds as $userId) { + $user = $this->userManager->get($userId); + if ($user) { + $output->writeln("Remove deleted files of $userId"); + $this->expireTrashForUser($user, $output); + $output->writeln("Unknown user $userId"); + return 1; } else { - $output->writeln("Unknown user $user"); + $output->writeln("Unknown user $userId"); return 1; } } @@ -71,7 +72,7 @@ class ExpireTrash extends Command { $users = $this->userManager->getSeenUsers(); foreach ($users as $user) { $p->advance(); - $this->expireTrashForUser($user); + $this->expireTrashForUser($user, $output); } $p->finish(); $output->writeln(''); @@ -79,33 +80,25 @@ class ExpireTrash extends Command { return 0; } - public function expireTrashForUser(IUser $user) { + private function expireTrashForUser(IUser $user, OutputInterface $output): void { try { - $uid = $user->getUID(); - if (!$this->setupFS($uid)) { - return; - } - Trashbin::expire($uid); + $trashRoot = $this->getTrashRoot($user); + Trashbin::expire($trashRoot, $user); } catch (\Throwable $e) { - $this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]); + $output->writeln('Error while expiring trashbin for user ' . $user->getUID() . ''); + throw $e; + } finally { + $this->setupManager->tearDown(); } } - /** - * Act on behalf on trash item owner - * @param string $user - * @return boolean - */ - protected function setupFS($user) { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($user); + private function getTrashRoot(IUser $user): Folder { + $this->setupManager->setupForUser($user); - // Check if this user has a trashbin directory - $view = new View('/' . $user); - if (!$view->is_dir('/files_trashbin/files')) { - return false; + $folder = $this->rootFolder->getUserFolder($user->getUID())->getParent()->get('files_trashbin'); + if (!$folder instanceof Folder) { + throw new \LogicException("Didn't expect files_trashbin to be a file instead of a folder"); } - - return true; + return $folder; } } diff --git a/apps/files_trashbin/lib/Command/RestoreAllFiles.php b/apps/files_trashbin/lib/Command/RestoreAllFiles.php index ce31f759c0e..938a98b36bf 100644 --- a/apps/files_trashbin/lib/Command/RestoreAllFiles.php +++ b/apps/files_trashbin/lib/Command/RestoreAllFiles.php @@ -7,6 +7,7 @@ namespace OCA\Files_Trashbin\Command; use OC\Core\Command\Base; +use OC\Files\SetupManager; use OCA\Files_Trashbin\Trash\ITrashManager; use OCA\Files_Trashbin\Trash\TrashItem; use OCP\Files\IRootFolder; @@ -14,6 +15,7 @@ use OCP\IDBConnection; use OCP\IL10N; use OCP\IUserBackend; use OCP\IUserManager; +use OCP\IUserSession; use OCP\L10N\IFactory; use Symfony\Component\Console\Exception\InvalidOptionException; use Symfony\Component\Console\Input\InputArgument; @@ -48,6 +50,8 @@ class RestoreAllFiles extends Base { protected IUserManager $userManager, protected IDBConnection $dbConnection, protected ITrashManager $trashManager, + protected SetupManager $setupManager, + protected IUserSession $userSession, IFactory $l10nFactory, ) { parent::__construct(); @@ -140,17 +144,16 @@ class RestoreAllFiles extends Base { * Restore deleted files for the given user according to the given filters */ protected function restoreDeletedFiles(string $uid, int $scope, ?int $since, ?int $until, bool $dryRun, OutputInterface $output): void { - \OC_Util::tearDownFS(); - \OC_Util::setupFS($uid); - \OC_User::setUserId($uid); - $user = $this->userManager->get($uid); - - if ($user === null) { + if (!$user) { $output->writeln("Unknown user $uid"); return; } + $this->setupManager->tearDown(); + $this->setupManager->setupForUser($user); + $this->userSession->setUser($user); + $userTrashItems = $this->filterTrashItems( $this->trashManager->listTrashRoot($user), $scope, diff --git a/apps/files_trashbin/lib/Command/Size.php b/apps/files_trashbin/lib/Command/Size.php index 9c19d4d92b3..f28511cace7 100644 --- a/apps/files_trashbin/lib/Command/Size.php +++ b/apps/files_trashbin/lib/Command/Size.php @@ -10,10 +10,12 @@ namespace OCA\Files_Trashbin\Command; use OC\Core\Command\Base; use OCP\Command\IBus; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\Util; +use Override; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -21,14 +23,16 @@ use Symfony\Component\Console\Output\OutputInterface; class Size extends Base { public function __construct( - private IConfig $config, - private IUserManager $userManager, - private IBus $commandBus, + private readonly IAppConfig $appConfig, + private readonly IConfig $config, + private readonly IUserManager $userManager, + private readonly IBus $commandBus, ) { parent::__construct(); } - protected function configure() { + #[Override] + protected function configure(): void { parent::configure(); $this ->setName('trashbin:size') @@ -41,6 +45,7 @@ class Size extends Base { ); } + #[Override] protected function execute(InputInterface $input, OutputInterface $output): int { $user = $input->getOption('user'); $size = $input->getArgument('size'); @@ -55,7 +60,7 @@ class Size extends Base { $this->config->setUserValue($user, 'files_trashbin', 'trashbin_size', (string)$parsedSize); $this->commandBus->push(new Expire($user)); } else { - $this->config->setAppValue('files_trashbin', 'trashbin_size', (string)$parsedSize); + $this->appConfig->setValueInt('files_trashbin', 'trashbin_size', $parsedSize); $output->writeln('Warning: changing the default trashbin size will automatically trigger cleanup of existing trashbins,'); $output->writeln('a users trashbin can exceed the configured size until they move a new file to the trashbin.'); } @@ -66,8 +71,8 @@ class Size extends Base { return 0; } - private function printTrashbinSize(InputInterface $input, OutputInterface $output, ?string $user) { - $globalSize = (int)$this->config->getAppValue('files_trashbin', 'trashbin_size', '-1'); + private function printTrashbinSize(InputInterface $input, OutputInterface $output, ?string $user): void { + $globalSize = $this->appConfig->getValueInt('files_trashbin', 'trashbin_size', -1); if ($globalSize < 0) { $globalHumanSize = 'default (50% of available space)'; } else { diff --git a/apps/files_trashbin/lib/Listener/EventListener.php b/apps/files_trashbin/lib/Listener/EventListener.php index 63ecc9c81f7..06f8f48e7bb 100644 --- a/apps/files_trashbin/lib/Listener/EventListener.php +++ b/apps/files_trashbin/lib/Listener/EventListener.php @@ -15,11 +15,18 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\Files\Events\BeforeFileSystemSetupEvent; use OCP\Files\Events\Node\NodeWrittenEvent; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; +use OCP\IUserManager; use OCP\User\Events\BeforeUserDeletedEvent; /** @template-implements IEventListener */ class EventListener implements IEventListener { public function __construct( + private IUserManager $userManager, + private IRootFolder $rootFolder, private ?string $userId = null, ) { } @@ -27,8 +34,19 @@ class EventListener implements IEventListener { public function handle(Event $event): void { if ($event instanceof NodeWrittenEvent) { // Resize trash - if (!empty($this->userId)) { - Trashbin::resizeTrash($this->userId); + if (empty($this->userId)) { + return; + } + try { + /** @var Folder $trashRoot */ + $trashRoot = $this->rootFolder->get('/' . $this->userId . '/files_trashbin'); + } catch (NotFoundException|NotPermittedException) { + return; + } + + $user = $this->userManager->get($this->userId); + if ($user) { + Trashbin::resizeTrash($trashRoot, $user); } } diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index e21da9b8cd0..8d1e13556e9 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -44,6 +44,7 @@ use OCP\IConfig; use OCP\IDBConnection; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; @@ -774,24 +775,19 @@ class Trashbin implements IEventListener { } /** - * calculate remaining free space for trash bin + * Calculate remaining free space for trash bin * * @param int|float $trashbinSize current size of the trash bin - * @param string $user * @return int|float available free space for trash bin */ - private static function calculateFreeSpace(int|float $trashbinSize, string $user): int|float { - $configuredTrashbinSize = static::getConfiguredTrashbinSize($user); + private static function calculateFreeSpace(Folder $userFolder, int|float $trashbinSize, IUser $user): int|float { + $configuredTrashbinSize = static::getConfiguredTrashbinSize($user->getUID()); if ($configuredTrashbinSize > -1) { return $configuredTrashbinSize - $trashbinSize; } - $userObject = Server::get(IUserManager::class)->get($user); - if (is_null($userObject)) { - return 0; - } $softQuota = true; - $quota = $userObject->getQuota(); + $quota = $user->getQuota(); if ($quota === null || $quota === 'none') { $quota = Filesystem::free_space('/'); $softQuota = false; @@ -810,10 +806,6 @@ class Trashbin implements IEventListener { // calculate available space for trash bin // subtract size of files and current trash bin size from quota if ($softQuota) { - $userFolder = \OC::$server->getUserFolder($user); - if (is_null($userFolder)) { - return 0; - } $free = $quota - $userFolder->getSize(false); // remaining free space for user if ($free > 0) { $availableSpace = ($free * self::DEFAULTMAXSIZE / 100) - $trashbinSize; // how much space can be used for versions @@ -828,38 +820,34 @@ class Trashbin implements IEventListener { } /** - * resize trash bin if necessary after a new file was added to Nextcloud - * - * @param string $user user id + * Resize trash bin if necessary after a new file was added to Nextcloud */ - public static function resizeTrash($user) { - $size = self::getTrashbinSize($user); + public static function resizeTrash(Folder $trashRoot, IUser $user): void { + $trashBinSize = $trashRoot->getSize(); - $freeSpace = self::calculateFreeSpace($size, $user); + $freeSpace = self::calculateFreeSpace($trashRoot->getParent(), $trashBinSize, $user); if ($freeSpace < 0) { - self::scheduleExpire($user); + self::scheduleExpire($user->getUID()); } } /** - * clean up the trash bin - * - * @param string $user + * Clean up the trash bin */ - public static function expire($user) { - $trashBinSize = self::getTrashbinSize($user); - $availableSpace = self::calculateFreeSpace($trashBinSize, $user); + public static function expire(Folder $trashRoot, IUser $user): void { + $trashBinSize = $trashRoot->getSize(); + $availableSpace = self::calculateFreeSpace($trashRoot->getParent(), $trashBinSize, $user); - $dirContent = Helper::getTrashFiles('/', $user, 'mtime'); + $dirContent = Helper::getTrashFiles('/', $user->getUID(), 'mtime'); // delete all files older then $retention_obligation - [$delSize, $count] = self::deleteExpiredFiles($dirContent, $user); + [$delSize, $count] = self::deleteExpiredFiles($dirContent, $user->getUID()); $availableSpace += $delSize; // delete files from trash until we meet the trash bin size limit again - self::deleteFiles(array_slice($dirContent, $count), $user, $availableSpace); + self::deleteFiles(array_slice($dirContent, $count), $user->getUID(), $availableSpace); } /** diff --git a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php index ec5e7162895..3ec8aa5894f 100644 --- a/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php +++ b/apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php @@ -14,6 +14,7 @@ use OCA\Files_Trashbin\BackgroundJob\ExpireTrash; use OCA\Files_Trashbin\Expiration; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\IJobList; +use OCP\Files\IRootFolder; use OCP\Files\ISetupManager; use OCP\IAppConfig; use OCP\IUserManager; @@ -31,6 +32,7 @@ class ExpireTrashTest extends TestCase { private ITimeFactory&MockObject $time; private ISetupManager&MockObject $setupManager; private ILockingProvider&MockObject $lockingProvider; + private IRootFolder&MockObject $rootFolder; protected function setUp(): void { parent::setUp(); @@ -42,6 +44,7 @@ class ExpireTrashTest extends TestCase { $this->logger = $this->createMock(LoggerInterface::class); $this->setupManager = $this->createMock(ISetupManager::class); $this->lockingProvider = $this->createMock(ILockingProvider::class); + $this->rootFolder = $this->createMock(IRootFolder::class); $this->time = $this->createMock(ITimeFactory::class); $this->time->method('getTime') @@ -68,6 +71,7 @@ class ExpireTrashTest extends TestCase { $this->logger, $this->setupManager, $this->lockingProvider, + $this->rootFolder, $this->time, ); $job->start($this->jobList); @@ -87,6 +91,7 @@ class ExpireTrashTest extends TestCase { $this->logger, $this->setupManager, $this->lockingProvider, + $this->rootFolder, $this->time, ); $job->start($this->jobList); diff --git a/apps/files_trashbin/tests/Command/CleanUpTest.php b/apps/files_trashbin/tests/Command/CleanUpTest.php index d9451a3e09a..325643e7269 100644 --- a/apps/files_trashbin/tests/Command/CleanUpTest.php +++ b/apps/files_trashbin/tests/Command/CleanUpTest.php @@ -8,9 +8,12 @@ declare(strict_types=1); */ namespace OCA\Files_Trashbin\Tests\Command; +use OC\Files\SetupManager; use OCA\Files_Trashbin\Command\CleanUp; use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; use OCP\IDBConnection; +use OCP\IUser; use OCP\IUserManager; use OCP\Server; use OCP\UserInterface; @@ -34,16 +37,22 @@ class CleanUpTest extends TestCase { protected IDBConnection $dbConnection; protected CleanUp $cleanup; protected string $trashTable = 'files_trash'; - protected string $user0 = 'user0'; + protected IUser&MockObject $user0; + protected SetupManager&MockObject $setupManager; protected function setUp(): void { parent::setUp(); + + $this->user0 = $this->createMock(IUser::class); + $this->user0->method('getUID')->willReturn('user0'); + $this->rootFolder = $this->createMock(IRootFolder::class); $this->userManager = $this->createMock(IUserManager::class); $this->dbConnection = Server::get(IDBConnection::class); + $this->setupManager = $this->createMock(SetupManager::class); - $this->cleanup = new CleanUp($this->rootFolder, $this->userManager, $this->dbConnection); + $this->cleanup = new CleanUp($this->rootFolder, $this->userManager, $this->dbConnection, $this->setupManager); } /** @@ -74,17 +83,20 @@ class CleanUpTest extends TestCase { $this->initTable(); $this->rootFolder ->method('nodeExists') - ->with('/' . $this->user0 . '/files_trashbin') - ->willReturnOnConsecutiveCalls($nodeExists, false); + ->with('/' . $this->user0->getUID() . '/files_trashbin') + ->willReturn(false); if ($nodeExists) { $this->rootFolder ->method('get') - ->with('/' . $this->user0 . '/files_trashbin') + ->with('/' . $this->user0->getUID() . '/files_trashbin') ->willReturn($this->rootFolder); $this->rootFolder ->method('delete'); } else { - $this->rootFolder->expects($this->never())->method('get'); + $this->rootFolder + ->method('get') + ->with('/' . $this->user0->getUID() . '/files_trashbin') + ->willThrowException(new NotFoundException()); $this->rootFolder->expects($this->never())->method('delete'); } self::invokePrivate($this->cleanup, 'removeDeletedFiles', [$this->user0, new NullOutput(), false]); @@ -129,15 +141,19 @@ class CleanUpTest extends TestCase { $userIds = ['user1', 'user2', 'user3']; $instance = $this->getMockBuilder(CleanUp::class) ->onlyMethods(['removeDeletedFiles']) - ->setConstructorArgs([$this->rootFolder, $this->userManager, $this->dbConnection]) + ->setConstructorArgs([$this->rootFolder, $this->userManager, $this->dbConnection, $this->setupManager]) ->getMock(); $instance->expects($this->exactly(count($userIds))) ->method('removeDeletedFiles') - ->willReturnCallback(function ($user) use ($userIds): void { - $this->assertTrue(in_array($user, $userIds)); + ->willReturnCallback(function (IUser $user) use ($userIds): void { + $this->assertTrue(in_array($user->getUID(), $userIds)); }); $this->userManager->expects($this->exactly(count($userIds))) - ->method('userExists')->willReturn(true); + ->method('get')->willReturnCallback(function (string $userId): IUser { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($userId); + return $user; + }); $inputInterface = $this->createMock(\Symfony\Component\Console\Input\InputInterface::class); $inputInterface->method('getArgument') ->with('user_id') @@ -159,7 +175,7 @@ class CleanUpTest extends TestCase { $backendUsers = ['user1', 'user2']; $instance = $this->getMockBuilder(CleanUp::class) ->onlyMethods(['removeDeletedFiles']) - ->setConstructorArgs([$this->rootFolder, $this->userManager, $this->dbConnection]) + ->setConstructorArgs([$this->rootFolder, $this->userManager, $this->dbConnection, $this->setupManager]) ->getMock(); $backend = $this->createMock(UserInterface::class); $backend->method('getUsers') @@ -167,8 +183,8 @@ class CleanUpTest extends TestCase { ->willReturn($backendUsers); $instance->expects($this->exactly(count($backendUsers))) ->method('removeDeletedFiles') - ->willReturnCallback(function ($user) use ($backendUsers): void { - $this->assertTrue(in_array($user, $backendUsers)); + ->willReturnCallback(function (IUser $user) use ($backendUsers): void { + $this->assertTrue(in_array($user->getUID(), $backendUsers)); }); $inputInterface = $this->createMock(InputInterface::class); $inputInterface->method('getArgument') diff --git a/apps/files_trashbin/tests/Command/ExpireTrashTest.php b/apps/files_trashbin/tests/Command/ExpireTrashTest.php index 366c27cf1e6..55859da0557 100644 --- a/apps/files_trashbin/tests/Command/ExpireTrashTest.php +++ b/apps/files_trashbin/tests/Command/ExpireTrashTest.php @@ -6,6 +6,7 @@ */ namespace OCA\Files_Trashbin\Tests\Command; +use OC\Files\SetupManager; use OCA\Files_Trashbin\Command\ExpireTrash; use OCA\Files_Trashbin\Expiration; use OCA\Files_Trashbin\Helper; @@ -16,9 +17,9 @@ use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; use OCP\Server; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Log\LoggerInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; @@ -38,7 +39,6 @@ class ExpireTrashTest extends TestCase { private IUser $user; private ITimeFactory&MockObject $timeFactory; - protected function setUp(): void { parent::setUp(); @@ -66,7 +66,7 @@ class ExpireTrashTest extends TestCase { parent::tearDown(); } - #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'retentionObligationProvider')] + #[DataProvider(methodName: 'retentionObligationProvider')] public function testRetentionObligation(string $obligation, string $quota, int $elapsed, int $fileSize, bool $shouldExpire): void { $this->config->setSystemValues(['trashbin_retention_obligation' => $obligation]); $this->expiration->setRetentionObligation($obligation); @@ -99,9 +99,10 @@ class ExpireTrashTest extends TestCase { ->willReturn([$userId]); $command = new ExpireTrash( - Server::get(LoggerInterface::class), Server::get(IUserManager::class), - $this->expiration + $this->expiration, + Server::get(SetupManager::class), + Server::get(IRootFolder::class), ); $this->invokePrivate($command, 'execute', [$inputInterface, $outputInterface]); diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 4b141d200f1..6182ff28e44 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1707,72 +1707,18 @@ - - - getUID())]]> - - - - getUID())]]> - - - - - - - - - - - - - user)]]> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1850,7 +1796,6 @@ $targetPath, 'trashPath' => $sourcePath])]]> -