Merge pull request #55738 from nextcloud/carl/cleanup-preview-command

fix(preview-cleanup): Also delete previews stored in the oc_previews table

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());
 

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++;

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));
+	}
 }