Merge pull request #51911 from nextcloud/backport/51861/stable30

[stable30] fix: Catch exceptions when expiring trashbin

Author: artonge

apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php

@@ -13,13 +13,15 @@
 use OCP\BackgroundJob\TimedJob;
 use OCP\IAppConfig;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class ExpireTrash extends TimedJob {
 
 	public function __construct(
 		private IAppConfig $appConfig,
 		private IUserManager $userManager,
 		private Expiration $expiration,
+		private LoggerInterface $logger,
 		ITimeFactory $time
 	) {
 		parent::__construct($time);
@@ -43,12 +45,16 @@ protected function run($argument) {
 		$users = $this->userManager->getSeenUsers($offset);
 
 		foreach ($users as $user) {
-			$uid = $user->getUID();
-			if (!$this->setupFS($uid)) {
-				continue;
+			try {
+				$uid = $user->getUID();
+				if (!$this->setupFS($uid)) {
+					continue;
+				}
+				$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
+				Trashbin::deleteExpiredFiles($dirContent, $uid);
+			} catch (\Throwable $e) {
+				$this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
 			}
-			$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
-			Trashbin::deleteExpiredFiles($dirContent, $uid);
 
 			$offset++;
 

apps/files_trashbin/lib/Command/ExpireTrash.php

@@ -11,6 +11,7 @@
 use OCA\Files_Trashbin\Trashbin;
 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;
@@ -33,7 +34,8 @@ class ExpireTrash extends Command {
 	 * @param IUserManager|null $userManager
 	 * @param Expiration|null $expiration
 	 */
-	public function __construct(?IUserManager $userManager = null,
+	public function __construct(private LoggerInterface $logger,
+		?IUserManager $userManager = null,
 		?Expiration $expiration = null) {
 		parent::__construct();
 
@@ -74,23 +76,29 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		} else {
 			$p = new ProgressBar($output);
 			$p->start();
-			$this->userManager->callForSeenUsers(function (IUser $user) use ($p) {
+
+			$users = $this->userManager->getSeenUsers();
+			foreach ($users as $user) {
 				$p->advance();
 				$this->expireTrashForUser($user);
-			});
+			}
 			$p->finish();
 			$output->writeln('');
 		}
 		return 0;
 	}
 
 	public function expireTrashForUser(IUser $user) {
-		$uid = $user->getUID();
-		if (!$this->setupFS($uid)) {
-			return;
+		try {
+			$uid = $user->getUID();
+			if (!$this->setupFS($uid)) {
+				return;
+			}
+			$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
+			Trashbin::deleteExpiredFiles($dirContent, $uid);
+		} catch (\Throwable $e) {
+			$this->logger->error('Error while expiring trashbin for user ' . $user->getUID(), ['exception' => $e]);
 		}
-		$dirContent = Helper::getTrashFiles('/', $uid, 'mtime');
-		Trashbin::deleteExpiredFiles($dirContent, $uid);
 	}
 
 	/**

apps/files_trashbin/tests/BackgroundJob/ExpireTrashTest.php

@@ -14,6 +14,7 @@
 use OCP\IAppConfig;
 use OCP\IUserManager;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class ExpireTrashTest extends TestCase {
@@ -29,6 +30,9 @@ class ExpireTrashTest extends TestCase {
 	/** @var IJobList&MockObject */
 	private $jobList;
 
+	/** @var LoggerInterface&MockObject */
+	private $logger;
+
 	/** @var ITimeFactory&MockObject */
 	private $time;
 
@@ -39,6 +43,7 @@ protected function setUp(): void {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->expiration = $this->createMock(Expiration::class);
 		$this->jobList = $this->createMock(IJobList::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 
 		$this->time = $this->createMock(ITimeFactory::class);
 		$this->time->method('getTime')
@@ -58,7 +63,7 @@ public function testConstructAndRun(): void {
 			->with('files_trashbin', 'background_job_expire_trash_offset', 0)
 			->willReturn(0);
 
-		$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->time);
+		$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
 		$job->start($this->jobList);
 	}
 
@@ -69,7 +74,7 @@ public function testBackgroundJobDeactivated(): void {
 		$this->expiration->expects($this->never())
 			->method('getMaxAgeAsTimestamp');
 
-		$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->time);
+		$job = new ExpireTrash($this->appConfig, $this->userManager, $this->expiration, $this->logger, $this->time);
 		$job->start($this->jobList);
 	}
 }