diff --git a/apps/files_trashbin/lib/Command/ExpireTrash.php b/apps/files_trashbin/lib/Command/ExpireTrash.php
index 1dc7bfc06893d..f541ff5651fa8 100644
--- a/apps/files_trashbin/lib/Command/ExpireTrash.php
+++ b/apps/files_trashbin/lib/Command/ExpireTrash.php
@@ -14,8 +14,10 @@
use OCA\Files_Trashbin\Trashbin;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
use OCP\IUser;
use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
@@ -28,6 +30,7 @@ public function __construct(
private readonly ?Expiration $expiration,
private readonly SetupManager $setupManager,
private readonly IRootFolder $rootFolder,
+ private readonly LoggerInterface $logger,
) {
parent::__construct();
}
@@ -66,8 +69,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
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 $userId");
return 1;
@@ -91,19 +92,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int
private function expireTrashForUser(IUser $user, OutputInterface $output): void {
try {
$trashRoot = $this->getTrashRoot($user);
+ if ($trashRoot === null) {
+ $output->writeln('No trashbin found for user ' . $user->getUID() . ', skipping', OutputInterface::VERBOSITY_VERBOSE);
+ return;
+ }
Trashbin::expire($trashRoot, $user);
} catch (\Throwable $e) {
$output->writeln('Error while expiring trashbin for user ' . $user->getUID() . '');
- throw $e;
+ $this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
} finally {
$this->setupManager->tearDown();
}
}
- private function getTrashRoot(IUser $user): Folder {
+ private function getTrashRoot(IUser $user): ?Folder {
$this->setupManager->setupForUser($user);
- $folder = $this->rootFolder->getUserFolder($user->getUID())->getParent()->get('files_trashbin');
+ try {
+ $folder = $this->rootFolder->getUserFolder($user->getUID())->getParent()->get('files_trashbin');
+ } catch (NotFoundException) {
+ return null;
+ }
if (!$folder instanceof Folder) {
throw new \LogicException("Didn't expect files_trashbin to be a file instead of a folder");
}
diff --git a/apps/files_trashbin/tests/Command/ExpireTrashTest.php b/apps/files_trashbin/tests/Command/ExpireTrashTest.php
index 6318be2450496..70cbf0102ad1d 100644
--- a/apps/files_trashbin/tests/Command/ExpireTrashTest.php
+++ b/apps/files_trashbin/tests/Command/ExpireTrashTest.php
@@ -21,6 +21,7 @@
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;
@@ -39,6 +40,7 @@ class ExpireTrashTest extends TestCase {
private IUserManager $userManager;
private IUser $user;
private ITimeFactory&MockObject $timeFactory;
+ private LoggerInterface&MockObject $logger;
protected function setUp(): void {
parent::setUp();
@@ -47,6 +49,7 @@ protected function setUp(): void {
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->expiration = Server::get(Expiration::class);
$this->invokePrivate($this->expiration, 'timeFactory', [$this->timeFactory]);
+ $this->logger = $this->createMock(LoggerInterface::class);
$userId = self::getUniqueID('user');
$this->userManager = Server::get(IUserManager::class);
@@ -92,24 +95,41 @@ public function testRetentionObligation(string $obligation, string $quota, int $
$trashFiles = Helper::getTrashFiles('/', $userId);
$this->assertEquals(1, count($trashFiles));
- $outputInterface = $this->createMock(OutputInterface::class);
- $inputInterface = $this->createMock(InputInterface::class);
- $inputInterface->expects($this->any())
- ->method('getArgument')
- ->with('user_id')
- ->willReturn([$userId]);
+ $this->logger->expects($this->never())->method('error');
+ $this->assertSame(0, $this->executeCommand([$userId]));
+
+ $trashFiles = Helper::getTrashFiles('/', $userId);
+ $this->assertEquals($shouldExpire ? 0 : 1, count($trashFiles));
+ }
+
+ public function testUserWithoutTrashbinIsSkipped(): void {
+ $this->expiration->setRetentionObligation('auto');
+ $this->logger->expects($this->never())->method('error');
+
+ $this->assertSame(0, $this->executeCommand([$this->user->getUID()]));
+ }
+
+ public function testErrorDoesNotAbortRemainingUsers(): void {
+ $this->expiration->setRetentionObligation('auto');
+ $this->userFolder->getParent()->newFile('files_trashbin');
+ $this->logger->expects($this->exactly(2))->method('error');
+
+ $this->assertSame(0, $this->executeCommand([$this->user->getUID(), $this->user->getUID()]));
+ }
+
+ private function executeCommand(array $userIds): int {
+ $input = $this->createMock(InputInterface::class);
+ $input->method('getArgument')->with('user_id')->willReturn($userIds);
$command = new ExpireTrash(
Server::get(IUserManager::class),
$this->expiration,
Server::get(SetupManager::class),
Server::get(IRootFolder::class),
+ $this->logger,
);
- $this->invokePrivate($command, 'execute', [$inputInterface, $outputInterface]);
-
- $trashFiles = Helper::getTrashFiles('/', $userId);
- $this->assertEquals($shouldExpire ? 0 : 1, count($trashFiles));
+ return $this->invokePrivate($command, 'execute', [$input, $this->createMock(OutputInterface::class)]);
}
public static function retentionObligationProvider(): array {