diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index 4b2d0e9d337..ce52af86d16 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -18,6 +18,7 @@ use OCA\Text\Service\SessionService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +use OCP\Files\NotPermittedException; use OCP\Server; use Psr\Log\LoggerInterface; @@ -43,9 +44,15 @@ protected function run($argument): void { try { $this->documentService->resetDocument($document->getId()); } catch (DocumentHasUnsavedChangesException) { + // The saved file may not reference attachments that are only used in unsaved steps yet + continue; } $setupManager->tearDown(); - $this->attachmentService->cleanupAttachments($document->getId()); + try { + $this->attachmentService->cleanupAttachments($document->getId()); + } catch (NotPermittedException $e) { + $this->logger->warning('Could not clean up attachments of file ' . $document->getId() . ', deleting is not permitted', ['exception' => $e]); + } } $this->logger->debug('Run cleanup job for text sessions'); diff --git a/tests/unit/CleanupTest.php b/tests/unit/CleanupTest.php new file mode 100644 index 00000000000..7a78d1b282f --- /dev/null +++ b/tests/unit/CleanupTest.php @@ -0,0 +1,104 @@ +document = Document::fromRow([ + 'id' => 123, + 'last_saved_version_time' => 1000, + ]); + $this->documentService = $this->createMock(DocumentService::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $attachmentService = new AttachmentService( + $this->rootFolder, + $this->createMock(IManager::class), + $this->createMock(IPreview::class), + $this->createMock(IMimeTypeDetector::class), + $this->createMock(IURLGenerator::class), + $this->createMock(IFilenameValidator::class), + $this->createMock(IFilesMetadataManager::class), + $this->createMock(ISession::class), + ); + $this->sessionService = $this->createMock(SessionService::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->documentService->expects(self::once()) + ->method('getAllWithNoActiveSession') + ->willReturn((function (): \Generator { + yield $this->document; + })()); + $this->cleanup = new class($this->createMock(ITimeFactory::class), $this->sessionService, $this->documentService, $attachmentService, $this->logger) extends Cleanup { + public function run($argument): void { + parent::run($argument); + } + }; + } + + private function expectRemainingCleanupSteps(): void { + $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'); + } + + public function testCleansAttachmentsOfResetDocument(): void { + $this->documentService->expects(self::once())->method('resetDocument')->with(123); + $this->rootFolder->expects(self::once()) + ->method('getFirstNodeById')->with(123) + ->willReturn(null); + $this->expectRemainingCleanupSteps(); + + $this->cleanup->run([]); + } + + public function testSkipsAttachmentCleanupForUnsavedChanges(): void { + $this->documentService->expects(self::once()) + ->method('resetDocument')->with(123) + ->willThrowException(new DocumentHasUnsavedChangesException()); + $this->rootFolder->expects(self::never())->method('getFirstNodeById'); + $this->expectRemainingCleanupSteps(); + + $this->cleanup->run([]); + } + + public function testNotPermittedCleanupIsLoggedAndJobContinues(): void { + $this->documentService->expects(self::once())->method('resetDocument')->with(123); + $this->rootFolder->expects(self::once()) + ->method('getFirstNodeById')->with(123) + ->willThrowException(new NotPermittedException()); + $this->logger->expects(self::once())->method('warning'); + $this->expectRemainingCleanupSteps(); + + $this->cleanup->run([]); + } +}