fix(preview): First cleanup from filecache and then from preview table

core/BackgroundJobs/PreviewMigrationJob.php

@@ -19,6 +19,7 @@
 use OCP\IConfig;
 use OCP\IDBConnection;
 use Override;
+use Psr\Log\LoggerInterface;
 
 class PreviewMigrationJob extends TimedJob {
 	private string $previewRootPath;
@@ -30,6 +31,7 @@ public function __construct(
 		private readonly IDBConnection $connection,
 		private readonly IRootFolder $rootFolder,
 		private readonly PreviewMigrationService $migrationService,
+		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($time);
 
@@ -95,11 +97,23 @@ private function processQueryResult(IResult $result): bool {
 		}
 
 		foreach ($fileIds as $fileId) {
-			$this->migrationService->migrateFileId($fileId, flatPath: false);
+			try {
+				$this->migrationService->migrateFileId($fileId, flatPath: false);
+			} catch (\Exception $e) {
+				$this->logger->error('Failed to migrate preview with fileId: ' . $fileId . ' (hierarchical file structure)', [
+					'exception' => $e,
+				]);
+			}
 		}
 
 		foreach ($flatFileIds as $fileId) {
-			$this->migrationService->migrateFileId($fileId, flatPath: true);
+			try {
+				$this->migrationService->migrateFileId($fileId, flatPath: true);
+			} catch (\Exception $e) {
+				$this->logger->error('Failed to migrate preview with fileId: ' . $fileId . ' (legacy file structure)', [
+					'exception' => $e,
+				]);
+			}
 		}
 		return $foundPreview;
 	}

core/Command/Preview/Cleanup.php

@@ -39,11 +39,11 @@ protected function configure(): void {
 
 	#[\Override]
 	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);
 	}
 
 	/**

lib/private/Preview/PreviewMigrationService.php

@@ -93,8 +93,9 @@ public function migrateFileId(int $fileId, bool $flatPath): array {
 			->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) {
 			foreach ($previewFiles as $previewFile) {
@@ -108,10 +109,15 @@ public function migrateFileId(int $fileId, bool $flatPath): array {
 				$preview->generateId();
 				try {
 					$preview = $this->previewMapper->insert($preview);
-				} catch (Exception) {
+				} catch (Exception $e) {
+					if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+						throw $e;
+					}
+
+					$delete = $this->connection->getQueryBuilder();
 					// We already have this preview in the preview table, skip
-					$qb->delete('filecache')
-						->where($qb->expr()->eq('fileid', $qb->createNamedParameter($file->getId())))
+					$delete->delete('filecache')
+						->where($delete->expr()->eq('fileid', $delete->createNamedParameter($file->getId())))
 						->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId())
 						->executeStatement();
 					continue;
@@ -179,7 +185,11 @@ private function deleteFolder(string $path): void {
 				break;
 			}
 
-			$folder = $this->appData->getFolder($current);
+			try {
+				$folder = $this->appData->getFolder($current);
+			} catch (NotFoundException) {
+				break;
+			}
 			if (count($folder->getDirectoryListing()) !== 0) {
 				break;
 			}

tests/Core/Command/Preview/CleanupTest.php

@@ -81,8 +81,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')
@@ -112,8 +110,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')
@@ -150,8 +146,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')
@@ -188,14 +182,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));
 	}
 }

tests/lib/Preview/PreviewMigrationJobTest.php

@@ -131,7 +131,8 @@ public function testMigrationLegacyPath(): void {
 				$this->previewMapper,
 				$this->storageFactory,
 				Server::get(IAppDataFactory::class),
-			)
+			),
+			$this->logger,
 		);
 		$this->invokePrivate($job, 'run', [[]]);
 		$this->assertEquals(0, count($this->previewAppData->getDirectoryListing()));
@@ -168,7 +169,8 @@ public function testMigrationPath(): void {
 				$this->previewMapper,
 				$this->storageFactory,
 				Server::get(IAppDataFactory::class),
-			)
+			),
+			$this->logger,
 		);
 		$this->invokePrivate($job, 'run', [[]]);
 		$this->assertEquals(0, count($this->previewAppData->getDirectoryListing()));
@@ -213,7 +215,8 @@ public function testMigrationPathWithVersion(): void {
 				$this->previewMapper,
 				$this->storageFactory,
 				Server::get(IAppDataFactory::class),
-			)
+			),
+			$this->logger,
 		);
 		$this->invokePrivate($job, 'run', [[]]);
 		$previews = iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5));