diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index a9a5c9f773c29..842aa56b1f217 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); @@ -49,10 +51,14 @@ protected function run(mixed $argument): void { $qb = $this->connection->getQueryBuilder(); $qb->select('path') ->from('filecache') - // Hierarchical preview folder structure - ->where($qb->expr()->like('path', $qb->createNamedParameter($this->previewRootPath . '%/%/%/%/%/%/%/%/%'))) - // Legacy flat preview folder structure - ->orWhere($qb->expr()->like('path', $qb->createNamedParameter($this->previewRootPath . '%/%.%'))) + ->where($qb->expr()->orX( + // Hierarchical preview folder structure + $qb->expr()->like('path', $qb->createNamedParameter($this->previewRootPath . '%/%/%/%/%/%/%/%/%')), + // Legacy flat preview folder structure + $qb->expr()->like('path', $qb->createNamedParameter($this->previewRootPath . '%/%.%')) + ))->andWhere( + $qb->expr()->eq('storage', $qb->createNamedParameter($this->rootFolder->getMountPoint()->getNumericStorageId())) + ) ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()) ->setMaxResults(100); @@ -95,11 +101,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..0fe6efbcc1e46 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); } /** @@ -77,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()) { @@ -102,16 +101,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/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/lib/public/Migration/Attributes/IndexMigrationAttribute.php b/lib/public/Migration/Attributes/IndexMigrationAttribute.php index b640a900f39d6..a67642cc726c1 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 5d9d51026a2b6..579d5abf6d23d 100644 --- a/tests/Core/Command/Preview/CleanupTest.php +++ b/tests/Core/Command/Preview/CleanupTest.php @@ -55,7 +55,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') @@ -66,13 +65,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' }; })); @@ -80,8 +78,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') @@ -92,7 +88,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') @@ -111,8 +106,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') @@ -124,7 +117,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') @@ -148,53 +140,16 @@ 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') - ->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 { + $this->rootFolder->method('getAppDataDirectoryName') + ->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->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));