From f957a70a98f1a5493fcfb7d20105654f919c4632 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 5 Jun 2026 12:06:48 +0200 Subject: [PATCH 1/6] fix(preview): First cleanup from filecache and then from preview table Signed-off-by: Carl Schwan --- core/Command/Preview/Cleanup.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/Command/Preview/Cleanup.php b/core/Command/Preview/Cleanup.php index b4cab0b1c9c88..d9c4d4b05436c 100644 --- a/core/Command/Preview/Cleanup.php +++ b/core/Command/Preview/Cleanup.php @@ -37,11 +37,11 @@ protected function configure(): void { } protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->deletePreviewFromPreviewTable($output) !== 0) { + if ($this->deletePreviewFromFileCacheTable($output) !== 0) { return 1; } - return $this->deletePreviewFromFileCacheTable($output); + return $this->deletePreviewFromPreviewTable($output); } /** From 76e382ab88ba7045d19f2e11c4456e50cae8851f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 8 Jun 2026 10:58:05 +0200 Subject: [PATCH 2/6] fix(preview): Don't reuse same query builder for delete query Signed-off-by: Carl Schwan From 9c2712465d6b7d5e98e70b2ab4b79b61eed1a95a Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 8 Jun 2026 11:08:02 +0200 Subject: [PATCH 3/6] fix(preview): Better handle errors while migrating previews Signed-off-by: Carl Schwan --- lib/private/Preview/PreviewMigrationService.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 29893363b38f3..377950da84952 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -94,8 +94,9 @@ public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = nul ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) ->setMaxResults(1); - $result = $qb->executeQuery(); - $result = $result->fetchAssociative(); + $cursor = $qb->executeQuery(); + $result = $cursor->fetchAssociative(); + $cursor->closeCursor(); if ($result !== false) { $oldFileIdsToDelete = []; From 6c59bb9ecbec576201f653f29b1f045b81c6fad7 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Mon, 8 Jun 2026 11:22:49 +0200 Subject: [PATCH 4/6] fix(preview): Adapt cleanup tests Now the logic is inverted so the tests need to be changed Signed-off-by: Carl Schwan --- tests/Core/Command/Preview/CleanupTest.php | 39 +++++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/tests/Core/Command/Preview/CleanupTest.php b/tests/Core/Command/Preview/CleanupTest.php index f1373eff48871..c0c448d851341 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/tests/Core/Command/Preview/CleanupTest.php @@ -79,8 +79,6 @@ public function testCleanup(): void { } public function testCleanupWhenNotDeletable(): void { - $this->previewService->expects($this->once())->method('deleteAll'); - $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -110,8 +108,6 @@ public function testCleanupWhenNotDeletable(): void { #[\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') @@ -148,8 +144,6 @@ public static function dataForTestCleanupWithDeleteException(): array { } public function testCleanupWithCreateException(): void { - $this->previewService->expects($this->once())->method('deleteAll'); - $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) ->method('isDeletable') @@ -186,14 +180,41 @@ public function testCleanupWithCreateException(): void { } public function testCleanupWithPreviewServiceException(): void { + $previewFolder = $this->createMock(Folder::class); + $previewFolder->expects($this->once()) + ->method('isDeletable') + ->willReturn(true); + + $previewFolder->expects($this->once()) + ->method('delete'); + + $appDataFolder = $this->createMock(Folder::class); + $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); + $appDataFolder->expects($this->once())->method('newFolder')->with('preview'); + + $this->rootFolder->method('getAppDataDirectoryName') + ->willReturn('appdata_some_id'); + + $this->rootFolder->method('get') + ->with('appdata_some_id') + ->willReturn($appDataFolder); + + $this->output->expects($this->exactly(3))->method('writeln') + ->with(self::callback(function (string $message): bool { + static $i = 0; + return match (++$i) { + 1 => $message === 'Preview folder deleted', + 2 => $message === 'Preview folder recreated', + 3 => $message === 'Previews removed' + }; + })); + + $this->assertEquals(0, $this->repair->run($this->input, $this->output)); $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)); } } From a409b05ad924c1d246edd840749bd2e346e02fef Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 9 Jun 2026 11:14:39 +0200 Subject: [PATCH 5/6] fix(preview): Do not recreate preview folder in filecache after cleanup Signed-off-by: Carl Schwan --- core/Command/Preview/Cleanup.php | 10 ---- .../Attributes/IndexMigrationAttribute.php | 4 +- tests/Core/Command/Preview/CleanupTest.php | 51 ++----------------- 3 files changed, 6 insertions(+), 59 deletions(-) diff --git a/core/Command/Preview/Cleanup.php b/core/Command/Preview/Cleanup.php index d9c4d4b05436c..cf4d1e08e8296 100644 --- a/core/Command/Preview/Cleanup.php +++ b/core/Command/Preview/Cleanup.php @@ -100,16 +100,6 @@ private function deletePreviewFromFileCacheTable(OutputInterface $output): int { return 1; } - try { - $appDataFolder->newFolder('preview'); - $this->logger->debug('Preview folder recreated'); - $output->writeln('Preview folder recreated', OutputInterface::VERBOSITY_VERBOSE); - } catch (NotPermittedException $e) { - $output->writeln("Preview folder was deleted, but you don't have the permission to create preview folder"); - $this->logger->error("Preview folder was deleted, but you don't have the permission to create preview folder", ['exception' => $e]); - return 1; - } - $output->writeln('Previews removed'); return 0; } diff --git a/lib/public/Migration/Attributes/IndexMigrationAttribute.php b/lib/public/Migration/Attributes/IndexMigrationAttribute.php index 31b9e2da2778e..ddf009358256a 100644 --- a/lib/public/Migration/Attributes/IndexMigrationAttribute.php +++ b/lib/public/Migration/Attributes/IndexMigrationAttribute.php @@ -11,7 +11,7 @@ use OCP\AppFramework\Attribute\Consumable; /** - * generic class related to migration attribute about index changes + * Generic class related to migration attribute about index changes */ #[Consumable(since: '30.0.0')] class IndexMigrationAttribute extends MigrationAttribute { @@ -19,7 +19,7 @@ class IndexMigrationAttribute extends MigrationAttribute { * @param string $table name of the database table * @param IndexType|null $type type of the index * @param string $description description of the migration - * @param array $notes notes abour the migration/index + * @param array $notes notes about the migration/index * @since 30.0.0 */ public function __construct( diff --git a/tests/Core/Command/Preview/CleanupTest.php b/tests/Core/Command/Preview/CleanupTest.php index c0c448d851341..178f8788ee4eb 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/tests/Core/Command/Preview/CleanupTest.php @@ -54,7 +54,6 @@ public function testCleanup(): void { $appDataFolder = $this->createMock(Folder::class); $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $appDataFolder->expects($this->once())->method('newFolder')->with('preview'); $this->rootFolder->expects($this->once()) ->method('getAppDataDirectoryName') @@ -65,13 +64,12 @@ public function testCleanup(): void { ->with('appdata_some_id') ->willReturn($appDataFolder); - $this->output->expects($this->exactly(3))->method('writeln') + $this->output->expects($this->exactly(2))->method('writeln') ->with(self::callback(function (string $message): bool { static $i = 0; return match (++$i) { 1 => $message === 'Preview folder deleted', - 2 => $message === 'Preview folder recreated', - 3 => $message === 'Previews removed' + 2 => $message === 'Previews removed' }; })); @@ -89,7 +87,6 @@ public function testCleanupWhenNotDeletable(): void { $appDataFolder = $this->createMock(Folder::class); $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $appDataFolder->expects($this->never())->method('newFolder')->with('preview'); $this->rootFolder->expects($this->once()) ->method('getAppDataDirectoryName') @@ -119,7 +116,6 @@ public function testCleanupWithDeleteException(string $exceptionClass, string $e $appDataFolder = $this->createMock(Folder::class); $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $appDataFolder->expects($this->never())->method('newFolder')->with('preview'); $this->rootFolder->expects($this->once()) ->method('getAppDataDirectoryName') @@ -143,42 +139,6 @@ public static function dataForTestCleanupWithDeleteException(): array { ]; } - public function testCleanupWithCreateException(): void { - $previewFolder = $this->createMock(Folder::class); - $previewFolder->expects($this->once()) - ->method('isDeletable') - ->willReturn(true); - - $previewFolder->expects($this->once()) - ->method('delete'); - - $appDataFolder = $this->createMock(Folder::class); - $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $appDataFolder->expects($this->once())->method('newFolder')->with('preview')->willThrowException(new NotPermittedException()); - - $this->rootFolder->expects($this->once()) - ->method('getAppDataDirectoryName') - ->willReturn('appdata_some_id'); - - $this->rootFolder->expects($this->once()) - ->method('get') - ->with('appdata_some_id') - ->willReturn($appDataFolder); - - $this->output->expects($this->exactly(2))->method('writeln') - ->with(self::callback(function (string $message): bool { - static $i = 0; - return match (++$i) { - 1 => $message === 'Preview folder deleted', - 2 => $message === "Preview folder was deleted, but you don't have the permission to create preview folder", - }; - })); - - $this->logger->expects($this->once())->method('error')->with("Preview folder was deleted, but you don't have the permission to create preview folder"); - - $this->assertEquals(1, $this->repair->run($this->input, $this->output)); - } - public function testCleanupWithPreviewServiceException(): void { $previewFolder = $this->createMock(Folder::class); $previewFolder->expects($this->once()) @@ -190,7 +150,6 @@ public function testCleanupWithPreviewServiceException(): void { $appDataFolder = $this->createMock(Folder::class); $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $appDataFolder->expects($this->once())->method('newFolder')->with('preview'); $this->rootFolder->method('getAppDataDirectoryName') ->willReturn('appdata_some_id'); @@ -199,17 +158,15 @@ public function testCleanupWithPreviewServiceException(): void { ->with('appdata_some_id') ->willReturn($appDataFolder); - $this->output->expects($this->exactly(3))->method('writeln') + $this->output->expects($this->exactly(2))->method('writeln') ->with(self::callback(function (string $message): bool { static $i = 0; return match (++$i) { 1 => $message === 'Preview folder deleted', - 2 => $message === 'Preview folder recreated', - 3 => $message === 'Previews removed' + 2 => $message === 'Previews removed' }; })); - $this->assertEquals(0, $this->repair->run($this->input, $this->output)); $this->previewService->expects($this->once())->method('deleteAll') ->willThrowException(new NotPermittedException('abc')); From 35257f07a803da27c497c00d4251f0646a5a39d6 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 9 Jun 2026 11:16:19 +0200 Subject: [PATCH 6/6] fix(preview): Don't abort cleanup of previews too early If we don't find previews in the filecache, this is now normal. Don't abort and instead delete previews from the new preview table instead. Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 1 + core/Command/Preview/Cleanup.php | 7 +++-- .../Attributes/IndexMigrationAttribute.php | 2 ++ tests/Core/Command/Preview/CleanupTest.php | 28 ++----------------- 4 files changed, 10 insertions(+), 28 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index e5bd2b4a49450..8ddb21966e495 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -9,6 +9,7 @@ namespace OC\Core\BackgroundJobs; +use OC\Preview\Db\Preview; use OC\Preview\PreviewMigrationService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; diff --git a/core/Command/Preview/Cleanup.php b/core/Command/Preview/Cleanup.php index cf4d1e08e8296..0fe6efbcc1e46 100644 --- a/core/Command/Preview/Cleanup.php +++ b/core/Command/Preview/Cleanup.php @@ -30,12 +30,14 @@ public function __construct( parent::__construct(); } + #[\Override] protected function configure(): void { $this ->setName('preview:cleanup') ->setDescription('Removes existing preview files'); } + #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { if ($this->deletePreviewFromFileCacheTable($output) !== 0) { return 1; @@ -75,9 +77,8 @@ private function deletePreviewFromFileCacheTable(OutputInterface $output): int { $previewFolder = $appDataFolder->get('preview'); } catch (NotFoundException $e) { - $this->logger->error("Previews can't be removed: appdata folder can't be found", ['exception' => $e]); - $output->writeln("Previews can't be removed: preview folder isn't deletable"); - return 1; + $this->logger->info("Legacy previews can't be removed: appdata folder can't be found", ['exception' => $e]); + return 0; } if (!$previewFolder->isDeletable()) { diff --git a/lib/public/Migration/Attributes/IndexMigrationAttribute.php b/lib/public/Migration/Attributes/IndexMigrationAttribute.php index ddf009358256a..a67642cc726c1 100644 --- a/lib/public/Migration/Attributes/IndexMigrationAttribute.php +++ b/lib/public/Migration/Attributes/IndexMigrationAttribute.php @@ -56,6 +56,7 @@ public function getType(): ?IndexType { * @return $this * @since 30.0.0 */ + #[\Override] public function import(array $data): self { parent::import($data); $this->setType(IndexType::tryFrom($data['type'] ?? '')); @@ -66,6 +67,7 @@ public function import(array $data): self { * @return array * @since 30.0.0 */ + #[\Override] public function jsonSerialize(): array { return array_merge( parent::jsonSerialize(), diff --git a/tests/Core/Command/Preview/CleanupTest.php b/tests/Core/Command/Preview/CleanupTest.php index 178f8788ee4eb..579d5abf6d23d 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/tests/Core/Command/Preview/CleanupTest.php @@ -26,6 +26,7 @@ class CleanupTest extends TestCase { private PreviewService&MockObject $previewService; private Cleanup $repair; + #[\Override] protected function setUp(): void { parent::setUp(); $this->rootFolder = $this->createMock(IRootFolder::class); @@ -140,36 +141,13 @@ public static function dataForTestCleanupWithDeleteException(): array { } public function testCleanupWithPreviewServiceException(): void { - $previewFolder = $this->createMock(Folder::class); - $previewFolder->expects($this->once()) - ->method('isDeletable') - ->willReturn(true); - - $previewFolder->expects($this->once()) - ->method('delete'); - - $appDataFolder = $this->createMock(Folder::class); - $appDataFolder->expects($this->once())->method('get')->with('preview')->willReturn($previewFolder); - $this->rootFolder->method('getAppDataDirectoryName') - ->willReturn('appdata_some_id'); - - $this->rootFolder->method('get') - ->with('appdata_some_id') - ->willReturn($appDataFolder); - - $this->output->expects($this->exactly(2))->method('writeln') - ->with(self::callback(function (string $message): bool { - static $i = 0; - return match (++$i) { - 1 => $message === 'Preview folder deleted', - 2 => $message === 'Previews removed' - }; - })); + ->willThrowException(new NotFoundException()); $this->previewService->expects($this->once())->method('deleteAll') ->willThrowException(new NotPermittedException('abc')); + $this->logger->expects($this->once())->method('info')->with("Legacy previews can't be removed: appdata folder can't be found"); $this->logger->expects($this->once())->method('error')->with("Previews can't be removed: exception occurred: abc"); $this->assertEquals(1, $this->repair->run($this->input, $this->output));