diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index a9a5c9f773c29..5a8f9fffbccac 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/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; } diff --git a/core/Command/Preview/Cleanup.php b/core/Command/Preview/Cleanup.php index 56508c3f0f024..ffed7a65ccd4a 100644 --- a/core/Command/Preview/Cleanup.php +++ b/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); } /** diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 6b67b8334772d..555bd3884474b 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/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; } diff --git a/tests/Core/Command/Preview/CleanupTest.php b/tests/Core/Command/Preview/CleanupTest.php index 84b0068842f6d..f321ac719c2cb 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/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)); } } diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index 2f769d79d24ed..fe9cd96879959 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/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));