diff --git a/core/Command/Preview/Cleanup.php b/core/Command/Preview/Cleanup.php index dad981a5243..b4cab0b1c9c 100644 --- a/core/Command/Preview/Cleanup.php +++ b/core/Command/Preview/Cleanup.php @@ -10,6 +10,8 @@ declare(strict_types=1); namespace OC\Core\Command\Preview; use OC\Core\Command\Base; +use OC\Preview\PreviewService; +use OCP\DB\Exception; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -23,6 +25,7 @@ class Cleanup extends Base { public function __construct( private IRootFolder $rootFolder, private LoggerInterface $logger, + private PreviewService $previewService, ) { parent::__construct(); } @@ -34,6 +37,31 @@ class Cleanup extends Base { } protected function execute(InputInterface $input, OutputInterface $output): int { + if ($this->deletePreviewFromPreviewTable($output) !== 0) { + return 1; + } + + return $this->deletePreviewFromFileCacheTable($output); + } + + /** + * Delete from the new oc_previews table. + */ + private function deletePreviewFromPreviewTable(OutputInterface $output): int { + try { + $this->previewService->deleteAll(); + return 0; + } catch (NotPermittedException|Exception $e) { + $this->logger->error("Previews can't be removed: exception occurred: " . $e->getMessage(), ['exception' => $e]); + $output->writeln("Previews can't be removed: " . $e->getMessage() . '. See the logs for more details.'); + return 1; + } + } + + /** + * Legacy in case there are still previews stored there. + */ + private function deletePreviewFromFileCacheTable(OutputInterface $output): int { try { $appDataFolder = $this->rootFolder->get($this->rootFolder->getAppDataDirectoryName()); diff --git a/lib/private/Preview/PreviewService.php b/lib/private/Preview/PreviewService.php index fb32976840a..c4beb67a521 100644 --- a/lib/private/Preview/PreviewService.php +++ b/lib/private/Preview/PreviewService.php @@ -13,6 +13,8 @@ namespace OC\Preview; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OC\Preview\Storage\StorageFactory; +use OCP\DB\Exception; +use OCP\Files\NotPermittedException; use OCP\IDBConnection; class PreviewService { @@ -23,6 +25,10 @@ class PreviewService { ) { } + /** + * @throws NotPermittedException + * @throws Exception + */ public function deletePreview(Preview $preview): void { $this->storageFactory->deletePreview($preview); $this->previewMapper->delete($preview); @@ -99,11 +105,19 @@ class PreviewService { return $this->previewMapper->getPreviewsForMimeTypes($mimeTypes); } + /** + * @throws NotPermittedException + * @throws Exception + */ public function deleteAll(): void { $lastId = 0; while (true) { $previews = $this->previewMapper->getPreviews($lastId, 1000); $i = 0; + + // FIXME: Should we use transaction here? Du to the I/O created when + // deleting the previews from the storage, which might be on a network + // This might take a non trivial amount of time where the DB is locked. foreach ($previews as $preview) { $this->deletePreview($preview); $i++; diff --git a/tests/Core/Command/Preview/CleanupTest.php b/tests/Core/Command/Preview/CleanupTest.php index e4a83246e5b..f1373eff488 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/tests/Core/Command/Preview/CleanupTest.php @@ -7,6 +7,7 @@ namespace Core\Command\Preview; use OC\Core\Command\Preview\Cleanup; +use OC\Preview\PreviewService; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -22,15 +23,18 @@ class CleanupTest extends TestCase { private LoggerInterface&MockObject $logger; private InputInterface&MockObject $input; private OutputInterface&MockObject $output; + private PreviewService&MockObject $previewService; private Cleanup $repair; protected function setUp(): void { parent::setUp(); $this->rootFolder = $this->createMock(IRootFolder::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->previewService = $this->createMock(PreviewService::class); $this->repair = new Cleanup( $this->rootFolder, $this->logger, + $this->previewService, ); $this->input = $this->createMock(InputInterface::class); @@ -38,6 +42,8 @@ class CleanupTest extends TestCase { } public function testCleanup(): void { + $this->previewService->expects($this->once())->method('deleteAll'); + $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -73,6 +79,8 @@ class CleanupTest extends TestCase { } public function testCleanupWhenNotDeletable(): void { + $this->previewService->expects($this->once())->method('deleteAll'); + $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -102,6 +110,8 @@ class CleanupTest extends TestCase { #[\PHPUnit\Framework\Attributes\DataProvider('dataForTestCleanupWithDeleteException')] public function testCleanupWithDeleteException(string $exceptionClass, string $errorMessage): void { + $this->previewService->expects($this->once())->method('deleteAll'); + $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -138,6 +148,8 @@ class CleanupTest extends TestCase { } public function testCleanupWithCreateException(): void { + $this->previewService->expects($this->once())->method('deleteAll'); + $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -172,4 +184,16 @@ class CleanupTest extends TestCase { $this->assertEquals(1, $this->repair->run($this->input, $this->output)); } + + public function testCleanupWithPreviewServiceException(): void { + $this->previewService->expects($this->once())->method('deleteAll') + ->willThrowException(new NotPermittedException('abc')); + + $this->logger->expects($this->once())->method('error')->with("Previews can't be removed: exception occurred: abc"); + + $this->rootFolder->expects($this->never()) + ->method('get'); + + $this->assertEquals(1, $this->repair->run($this->input, $this->output)); + } }