diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index 9df5549d3f4..043c8d037e0 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -15,6 +15,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; use OCP\Files\ISetupManager; +use OCP\Files\NotPermittedException; use Psr\Log\LoggerInterface; class Cleanup extends TimedJob { @@ -41,7 +42,12 @@ protected function run($argument): void { continue; } $this->setupManager->tearDown(); - $this->attachmentService->cleanupAttachments($document->getContextId()); + try { + $this->attachmentService->cleanupAttachments($document->getContextId()); + } catch (NotPermittedException $e) { + // Retrying will not help until the document is saved again, so mark it as cleaned up anyway + $this->logger->warning('Could not clean up attachments of file ' . $document->getContextId() . ', deleting is not permitted', ['exception' => $e]); + } $this->documentService->markAttachmentCleanup($document); } diff --git a/tests/unit/CleanupTest.php b/tests/unit/CleanupTest.php index 0c52542ac5d..59102b839f3 100644 --- a/tests/unit/CleanupTest.php +++ b/tests/unit/CleanupTest.php @@ -19,6 +19,7 @@ use OCP\Files\IRootFolder; use OCP\Files\ISetupManager; use OCP\Files\Node; +use OCP\Files\NotPermittedException; use OCP\FilesMetadata\IFilesMetadataManager; use OCP\IPreview; use OCP\ISession; @@ -34,6 +35,7 @@ class CleanupTest extends \PHPUnit\Framework\TestCase { private SessionService $sessionService; private IRootFolder $rootFolder; private ISetupManager $setupManager; + private LoggerInterface $logger; public function setUp(): void { $this->document = Document::fromRow([ @@ -57,13 +59,14 @@ public function setUp(): void { ); $this->sessionService = $this->createMock(SessionService::class); $this->setupManager = $this->createMock(ISetupManager::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->documentService->expects(self::never())->method('resetDocument'); $this->documentService->expects(self::once()) ->method('getAllPendingAttachmentCleanup') ->willReturn((function (): \Generator { yield $this->document; })()); - $this->cleanup = new class($this->createMock(ITimeFactory::class), $this->sessionService, $this->documentService, $this->attachmentService, $this->createMock(LoggerInterface::class), $this->setupManager, ) extends Cleanup { + $this->cleanup = new class($this->createMock(ITimeFactory::class), $this->sessionService, $this->documentService, $this->attachmentService, $this->logger, $this->setupManager, ) extends Cleanup { public function run($argument): void { parent::run($argument); } @@ -115,4 +118,19 @@ public function testFailedCleanupRemainsPending(): void { $this->cleanup->run([]); } + + public function testNotPermittedCleanupIsMarkedAndJobContinues(): void { + $this->documentService->method('hasUnsavedChanges')->willReturn(false); + $this->rootFolder->expects(self::once()) + ->method('getFirstNodeById')->with(123) + ->willThrowException(new NotPermittedException()); + $this->logger->expects(self::once())->method('warning'); + $this->documentService->expects(self::once())->method('markAttachmentCleanup')->with($this->document); + $this->sessionService->expects(self::once())->method('removeInactiveSessionsWithoutSteps'); + $this->sessionService->expects(self::once())->method('removeOldSessions'); + $this->sessionService->expects(self::once())->method('removeOrphanedSteps'); + $this->documentService->expects(self::once())->method('cleanupOldDocumentsFolders'); + + $this->cleanup->run([]); + } }