Merge pull request #51912 from nextcloud/backport/51861/stable31

[stable31] fix: Catch exceptions when expiring trashbin

Author: artonge

apps/files_trashbin/lib/BackgroundJob/ExpireTrash.php

@@ -14,12 +14,14 @@
 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

@@ -12,6 +12,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;
@@ -25,6 +26,7 @@ class ExpireTrash extends Command {
 	 * @param Expiration|null $expiration
 	 */
 	public function __construct(
+		private LoggerInterface $logger,
 		private ?IUserManager $userManager = null,
 		private ?Expiration $expiration = null,
 	) {
@@ -64,23 +66,29 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		} else {
 			$p = new ProgressBar($output);
 			$p->start();
-			$this->userManager->callForSeenUsers(function (IUser $user) use ($p): void {
+
+			$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);
 	}
 }