From c192331dc05aac2dd7146db9d15330a20bd7d48f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 10:36:26 +0200 Subject: [PATCH 1/6] perf(preview): Optimize retriving all previews from oc_filecache Previoulsy we used a PATH LIKE expression which is fine for small instances but doesn't scale for big instance (timeout). Manually moving accross the tree with getFolderContentsById is significantly faster as we can use the index and this also reuse common APIs from OCP/Files instead of directly manipulating the filecache with the query builder. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 103 +++++++----------- tests/lib/Preview/PreviewMigrationJobTest.php | 3 - 2 files changed, 41 insertions(+), 65 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index 5ca3de9286a35..9a9e64b9703b3 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -12,11 +12,10 @@ use OC\Preview\PreviewMigrationService; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; -use OCP\DB\IResult; +use OCP\Files\FileInfo; use OCP\Files\IRootFolder; use OCP\IAppConfig; use OCP\IConfig; -use OCP\IDBConnection; use Override; use Psr\Log\LoggerInterface; @@ -27,7 +26,6 @@ public function __construct( ITimeFactory $time, private readonly IAppConfig $appConfig, private readonly IConfig $config, - private readonly IDBConnection $connection, private readonly IRootFolder $rootFolder, private readonly PreviewMigrationService $migrationService, private readonly LoggerInterface $logger, @@ -45,79 +43,60 @@ protected function run(mixed $argument): void { return; } - $startTime = time(); - while (true) { - $qb = $this->connection->getQueryBuilder(); - $qb->select('path') - ->from('filecache') - ->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); - - $result = $qb->executeQuery(); - $foundPreviews = $this->processQueryResult($result); - - if (!$foundPreviews) { - break; - } + $storage = $this->rootFolder->getMountPoint()->getStorage(); + if ($storage === null) { + $this->appConfig->setValueBool('core', 'previewMovedDone', true); + return; + } - // Stop if execution time is more than one hour. - if (time() - $startTime > 3600) { - return; - } + $cache = $storage->getCache(); + $previewRootId = $cache->getId(rtrim($this->previewRootPath, '/')); + if ($previewRootId === -1) { + // No previews have ever been generated on this instance. + $this->appConfig->setValueBool('core', 'previewMovedDone', true); + return; } - $this->appConfig->setValueBool('core', 'previewMovedDone', true); - } + $startTime = time(); - private function processQueryResult(IResult $result): bool { - $foundPreview = false; - $fileIds = []; - $flatFileIds = []; - while ($row = $result->fetch()) { - $pathSplit = explode('/', $row['path']); - assert(count($pathSplit) >= 2); - $fileId = (int)$pathSplit[count($pathSplit) - 2]; - if (count($pathSplit) === 11) { - // Hierarchical structure - if (!in_array($fileId, $fileIds)) { - $fileIds[] = $fileId; - } - } else { - // Flat structure - if (!in_array($fileId, $flatFileIds)) { - $flatFileIds[] = $fileId; + // Walk the preview folder tree via the `parent` column, which is indexed on + // every supported database platform. + // + // Depth from the preview root tells us which structure a leaf folder holds: + // - depth 1: legacy flat structure, e.g. preview//.png + // - depth 8: hierarchical structure, e.g. preview/a/b/c/d/e/f/g//.png + $foldersToVisit = [[$previewRootId, '', 0]]; + + while ($foldersToVisit !== []) { + [$folderId, $folderName, $depth] = array_pop($foldersToVisit); + + $hasPreviewFiles = false; + foreach ($cache->getFolderContentsById($folderId) as $entry) { + if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) { + $foldersToVisit[] = [$entry->getId(), $entry->getName(), $depth + 1]; + } else { + $hasPreviewFiles = true; } } - $foundPreview = true; - } - foreach ($fileIds as $fileId) { - 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, - ]); + if (!$hasPreviewFiles || !ctype_digit($folderName)) { + continue; } - } - foreach ($flatFileIds as $fileId) { try { - $this->migrationService->migrateFileId($fileId, flatPath: true); + $this->migrationService->migrateFileId((int)$folderName, flatPath: $depth === 1); } catch (\Exception $e) { - $this->logger->error('Failed to migrate preview with fileId: ' . $fileId . ' (legacy file structure)', [ + $this->logger->error('Failed to migrate preview with fileId: ' . $folderName, [ 'exception' => $e, ]); } + + // Stop if execution time is more than one hour. + if (time() - $startTime > 3600) { + return; + } } - return $foundPreview; + + $this->appConfig->setValueBool('core', 'previewMovedDone', true); } } diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index fe9cd96879959..06108c954fb6c 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -119,7 +119,6 @@ public function testMigrationLegacyPath(): void { Server::get(ITimeFactory::class), $this->appConfig, $this->config, - Server::get(IDBConnection::class), Server::get(IRootFolder::class), new PreviewMigrationService( $this->config, @@ -157,7 +156,6 @@ public function testMigrationPath(): void { Server::get(ITimeFactory::class), $this->appConfig, $this->config, - Server::get(IDBConnection::class), Server::get(IRootFolder::class), new PreviewMigrationService( $this->config, @@ -203,7 +201,6 @@ public function testMigrationPathWithVersion(): void { Server::get(ITimeFactory::class), $this->appConfig, $this->config, - Server::get(IDBConnection::class), Server::get(IRootFolder::class), new PreviewMigrationService( $this->config, From 0525899e352b4b32a6a214d0844b2ff12f796c45 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 12:34:17 +0200 Subject: [PATCH 2/6] perf: Batch deletion of old file entries Faster than doing a commit after each file Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- .../Preview/PreviewMigrationService.php | 120 ++++++++++++------ 1 file changed, 79 insertions(+), 41 deletions(-) diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 555bd3884474b..f4ed37ce8840e 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -14,6 +14,7 @@ use OC\Preview\Db\PreviewMapper; use OC\Preview\Storage\StorageFactory; use OCP\DB\Exception; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\IAppData; use OCP\Files\IMimeTypeDetector; @@ -98,45 +99,42 @@ public function migrateFileId(int $fileId, bool $flatPath): array { $cursor->closeCursor(); if ($result !== false) { - foreach ($previewFiles as $previewFile) { - /** @var Preview $preview */ - $preview = $previewFile['preview']; - /** @var SimpleFile $file */ - $file = $previewFile['file']; - $preview->setStorageId($result['storage']); - $preview->setEtag($result['etag']); - $preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype'])); - $preview->generateId(); - try { - $preview = $this->previewMapper->insert($preview); - } catch (Exception $e) { - if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { - throw $e; + $oldFileIdsToDelete = []; + try { + foreach ($previewFiles as $previewFile) { + /** @var Preview $preview */ + $preview = $previewFile['preview']; + /** @var SimpleFile $file */ + $file = $previewFile['file']; + $preview->setStorageId($result['storage']); + $preview->setEtag($result['etag']); + $preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype'])); + $preview->generateId(); + try { + $preview = $this->previewMapper->insert($preview); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + + // We already have this preview in the preview table, skip + $oldFileIdsToDelete[] = $file->getId(); + continue; } - $delete = $this->connection->getQueryBuilder(); - // We already have this preview in the preview table, skip - $delete->delete('filecache') - ->where($delete->expr()->eq('fileid', $delete->createNamedParameter($file->getId()))) - ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()) - ->executeStatement(); - continue; - } + try { + $this->storageFactory->migratePreview($preview, $file); + // Do not call $file->delete() as this will also delete the file from the file system + } catch (\Exception $e) { + $this->previewMapper->delete($preview); + throw $e; + } - try { - $this->storageFactory->migratePreview($preview, $file); - $qb = $this->connection->getQueryBuilder(); - $qb->delete('filecache') - ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($file->getId()))) - ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()) - ->executeStatement(); - // Do not call $file->delete() as this will also delete the file from the file system - } catch (\Exception $e) { - $this->previewMapper->delete($preview); - throw $e; + $oldFileIdsToDelete[] = $file->getId(); + $previews[] = $preview; } - - $previews[] = $preview; + } finally { + $this->deleteOldFileCacheEntries($oldFileIdsToDelete); } } else { // No matching fileId, delete preview @@ -165,6 +163,23 @@ private static function getInternalFolder(string $name, bool $flatPath): string return implode('/', str_split(substr(md5($name), 0, 7))) . '/' . $name; } + /** + * @param list $fileIds + */ + private function deleteOldFileCacheEntries(array $fileIds): void { + if ($fileIds === []) { + return; + } + + foreach (array_chunk($fileIds, 1000) as $chunk) { + $qb = $this->connection->getQueryBuilder(); + $qb->delete('filecache') + ->where($qb->expr()->in('fileid', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->hintShardKey('storage', $this->rootFolder->getMountPoint()->getNumericStorageId()) + ->executeStatement(); + } + } + private function deleteFolder(string $path): void { $current = $path; @@ -185,14 +200,37 @@ private function deleteFolder(string $path): void { break; } - try { - $folder = $this->appData->getFolder($current); - } catch (NotFoundException) { - break; - } - if (count($folder->getDirectoryListing()) !== 0) { + if ($this->folderHasChildren($rootFolderId, $this->previewRootPath . $current)) { break; } } } + + private function folderHasChildren(int $storageId, string $path): bool { + $qb = $this->connection->getQueryBuilder(); + $qb->select('fileid') + ->from('filecache') + ->where($qb->expr()->eq('path_hash', $qb->createNamedParameter(md5($path)))) + ->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId))) + ->setMaxResults(1); + $cursor = $qb->executeQuery(); + $folderId = $cursor->fetchOne(); + $cursor->closeCursor(); + + if ($folderId === false) { + // The folder itself is already gone, nothing to check. + return false; + } + + $qb = $this->connection->getQueryBuilder(); + $qb->select('fileid') + ->from('filecache') + ->where($qb->expr()->eq('parent', $qb->createNamedParameter((int)$folderId))) + ->setMaxResults(1); + $cursor = $qb->executeQuery(); + $hasChild = $cursor->fetchOne() !== false; + $cursor->closeCursor(); + + return $hasChild; + } } From 831d4a6f60609d79b94a988878cd86ae9c3dc91f Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 12:56:54 +0200 Subject: [PATCH 3/6] perf: Don't list items twice in the preview folder Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- core/BackgroundJobs/PreviewMigrationJob.php | 10 +-- .../Preview/PreviewMigrationService.php | 62 +++++++++---------- .../Preview/Storage/IPreviewStorage.php | 3 +- .../Preview/Storage/LocalPreviewStorage.php | 3 +- .../Storage/ObjectStorePreviewStorage.php | 3 +- .../Preview/Storage/StorageFactory.php | 5 +- 6 files changed, 41 insertions(+), 45 deletions(-) diff --git a/core/BackgroundJobs/PreviewMigrationJob.php b/core/BackgroundJobs/PreviewMigrationJob.php index 9a9e64b9703b3..e5bd2b4a49450 100644 --- a/core/BackgroundJobs/PreviewMigrationJob.php +++ b/core/BackgroundJobs/PreviewMigrationJob.php @@ -70,21 +70,23 @@ protected function run(mixed $argument): void { while ($foldersToVisit !== []) { [$folderId, $folderName, $depth] = array_pop($foldersToVisit); - $hasPreviewFiles = false; + // Collect the actual preview files here so migrateFileId() doesn't need to + // list this folder's contents a second time. + $previewEntries = []; foreach ($cache->getFolderContentsById($folderId) as $entry) { if ($entry->getMimeType() === FileInfo::MIMETYPE_FOLDER) { $foldersToVisit[] = [$entry->getId(), $entry->getName(), $depth + 1]; } else { - $hasPreviewFiles = true; + $previewEntries[] = $entry; } } - if (!$hasPreviewFiles || !ctype_digit($folderName)) { + if ($previewEntries === [] || !ctype_digit($folderName)) { continue; } try { - $this->migrationService->migrateFileId((int)$folderName, flatPath: $depth === 1); + $this->migrationService->migrateFileId((int)$folderName, flatPath: $depth === 1, entries: $previewEntries); } catch (\Exception $e) { $this->logger->error('Failed to migrate preview with fileId: ' . $folderName, [ 'exception' => $e, diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index f4ed37ce8840e..7be331362c441 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -16,6 +16,7 @@ use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\AppData\IAppDataFactory; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\IAppData; use OCP\Files\IMimeTypeDetector; use OCP\Files\IMimeTypeLoader; @@ -45,44 +46,43 @@ public function __construct( } /** - * @param array $previewFolders + * @param list|null $entries Preview file entries already fetched by the caller. * @return Preview[] */ - public function migrateFileId(int $fileId, bool $flatPath): array { + public function migrateFileId(int $fileId, bool $flatPath, ?array $entries = null): array { $previews = []; $internalPath = $this->getInternalFolder((string)$fileId, $flatPath); - try { - $folder = $this->appData->getFolder($internalPath); - } catch (NotFoundException) { - return []; + + if ($entries === null) { + try { + $entries = $this->appData->getFolder($internalPath)->getDirectoryListing(); + } catch (NotFoundException) { + return []; + } } /** - * @var list $previewFiles + * @var list $previewsToInsert */ - $previewFiles = []; + $previewsToInsert = []; - foreach ($folder->getDirectoryListing() as $previewFile) { - $path = $fileId . '/' . $previewFile->getName(); - /** @var SimpleFile $previewFile */ + foreach ($entries as $entry) { + $path = $fileId . '/' . $entry->getName(); $preview = Preview::fromPath($path, $this->mimeTypeDetector); if ($preview === false) { $this->logger->error('Unable to import old preview at path.'); continue; } $preview->generateId(); - $preview->setSize($previewFile->getSize()); - $preview->setMtime($previewFile->getMtime()); - $preview->setOldFileId($previewFile->getId()); + $preview->setSize($entry->getSize()); + $preview->setMtime($entry->getMTime()); + $preview->setOldFileId($entry->getId()); $preview->setEncrypted(false); - $previewFiles[] = [ - 'file' => $previewFile, - 'preview' => $preview, - ]; + $previewsToInsert[] = $preview; } - if (empty($previewFiles)) { + if (empty($previewsToInsert)) { $this->deleteFolder($internalPath); return $previews; @@ -101,11 +101,7 @@ public function migrateFileId(int $fileId, bool $flatPath): array { if ($result !== false) { $oldFileIdsToDelete = []; try { - foreach ($previewFiles as $previewFile) { - /** @var Preview $preview */ - $preview = $previewFile['preview']; - /** @var SimpleFile $file */ - $file = $previewFile['file']; + foreach ($previewsToInsert as $preview) { $preview->setStorageId($result['storage']); $preview->setEtag($result['etag']); $preview->setSourceMimeType($this->mimeTypeLoader->getMimetypeById((int)$result['mimetype'])); @@ -118,34 +114,36 @@ public function migrateFileId(int $fileId, bool $flatPath): array { } // We already have this preview in the preview table, skip - $oldFileIdsToDelete[] = $file->getId(); + $oldFileIdsToDelete[] = $preview->getOldFileId(); continue; } try { - $this->storageFactory->migratePreview($preview, $file); - // Do not call $file->delete() as this will also delete the file from the file system + $this->storageFactory->migratePreview($preview); + // Do not delete the old file via a Node here, as that would also + // delete it from the file system; only its filecache row is stale. } catch (\Exception $e) { $this->previewMapper->delete($preview); throw $e; } - $oldFileIdsToDelete[] = $file->getId(); + $oldFileIdsToDelete[] = $preview->getOldFileId(); $previews[] = $preview; } } finally { $this->deleteOldFileCacheEntries($oldFileIdsToDelete); } } else { - // No matching fileId, delete preview + // No matching fileId, delete the orphaned preview files themselves. try { + $folder = $this->appData->getFolder($internalPath); $this->connection->beginTransaction(); - foreach ($previewFiles as $previewFile) { - /** @var SimpleFile $file */ - $file = $previewFile['file']; + foreach ($folder->getDirectoryListing() as $file) { $file->delete(); } $this->connection->commit(); + } catch (NotFoundException) { + // Folder already gone, nothing to clean up. } catch (Exception) { $this->connection->rollback(); } diff --git a/lib/private/Preview/Storage/IPreviewStorage.php b/lib/private/Preview/Storage/IPreviewStorage.php index f7b60b9cc9be3..28d2dc53ef577 100644 --- a/lib/private/Preview/Storage/IPreviewStorage.php +++ b/lib/private/Preview/Storage/IPreviewStorage.php @@ -10,7 +10,6 @@ namespace OC\Preview\Storage; use Exception; -use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -42,7 +41,7 @@ public function deletePreview(Preview $preview): void; * To remove at some point * @throws Exception */ - public function migratePreview(Preview $preview, SimpleFile $file): void; + public function migratePreview(Preview $preview): void; /** * @throws NotPermittedException diff --git a/lib/private/Preview/Storage/LocalPreviewStorage.php b/lib/private/Preview/Storage/LocalPreviewStorage.php index 99427b74b2901..4d9d594089e40 100644 --- a/lib/private/Preview/Storage/LocalPreviewStorage.php +++ b/lib/private/Preview/Storage/LocalPreviewStorage.php @@ -12,7 +12,6 @@ use LogicException; use OC; -use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OCP\DB\Exception; @@ -93,7 +92,7 @@ private function createParentFiles(string $path): void { } #[Override] - public function migratePreview(Preview $preview, SimpleFile $file): void { + public function migratePreview(Preview $preview): void { // legacy flat directory $sourcePath = $this->getPreviewRootFolder() . $preview->getFileId() . '/' . $preview->getName(); if (!file_exists($sourcePath)) { diff --git a/lib/private/Preview/Storage/ObjectStorePreviewStorage.php b/lib/private/Preview/Storage/ObjectStorePreviewStorage.php index 713d39f9b0b38..f77da26eaea95 100644 --- a/lib/private/Preview/Storage/ObjectStorePreviewStorage.php +++ b/lib/private/Preview/Storage/ObjectStorePreviewStorage.php @@ -12,7 +12,6 @@ use Icewind\Streams\CountWrapper; use OC\Files\ObjectStore\PrimaryObjectStoreConfig; -use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OCP\Files\NotPermittedException; @@ -95,7 +94,7 @@ public function deletePreview(Preview $preview): void { } #[Override] - public function migratePreview(Preview $preview, SimpleFile $file): void { + public function migratePreview(Preview $preview): void { // Just set the Preview::bucket and Preview::objectStore $this->getObjectStoreInfoForNewPreview($preview, migration: true); $this->previewMapper->update($preview); diff --git a/lib/private/Preview/Storage/StorageFactory.php b/lib/private/Preview/Storage/StorageFactory.php index 69c7776c42e51..39cd11e09cfbd 100644 --- a/lib/private/Preview/Storage/StorageFactory.php +++ b/lib/private/Preview/Storage/StorageFactory.php @@ -10,7 +10,6 @@ namespace OC\Preview\Storage; use OC\Files\ObjectStore\PrimaryObjectStoreConfig; -use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; use OCP\Server; use Override; @@ -53,8 +52,8 @@ private function getBackend(): IPreviewStorage { } #[Override] - public function migratePreview(Preview $preview, SimpleFile $file): void { - $this->getBackend()->migratePreview($preview, $file); + public function migratePreview(Preview $preview): void { + $this->getBackend()->migratePreview($preview); } #[Override] From 1ebd8e3f5ec93e12e3dfc85789522f4c14451c18 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 13:07:15 +0200 Subject: [PATCH 4/6] chore(preview): Add more tests for the preview migration job Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Carl Schwan --- tests/lib/Preview/PreviewMigrationJobTest.php | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index 06108c954fb6c..c579ce092443c 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -10,6 +10,7 @@ namespace lib\Preview; use OC\Core\BackgroundJobs\PreviewMigrationJob; +use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OC\Preview\PreviewMigrationService; use OC\Preview\PreviewService; @@ -142,6 +143,128 @@ private static function getInternalFolder(string $name): string { return implode('/', str_split(substr(md5($name), 0, 7))) . '/' . $name; } + private function createJob(): PreviewMigrationJob { + return new PreviewMigrationJob( + Server::get(ITimeFactory::class), + $this->appConfig, + $this->config, + Server::get(IRootFolder::class), + new PreviewMigrationService( + $this->config, + Server::get(IRootFolder::class), + $this->logger, + $this->mimeTypeDetector, + $this->mimeTypeLoader, + Server::get(IDBConnection::class), + $this->previewMapper, + $this->storageFactory, + Server::get(IAppDataFactory::class), + ), + $this->logger, + ); + } + + private function insertFilecacheRow(string $path, string $etag): int { + $qb = $this->db->getQueryBuilder(); + $qb->insert('filecache') + ->values([ + 'storage' => $qb->createNamedParameter(1), + 'path' => $qb->createNamedParameter($path), + 'path_hash' => $qb->createNamedParameter(md5($path)), + 'parent' => $qb->createNamedParameter(0), + 'name' => $qb->createNamedParameter(basename($path)), + 'mimetype' => $qb->createNamedParameter(42), + 'size' => $qb->createNamedParameter(1000), + 'mtime' => $qb->createNamedParameter(1000), + 'storage_mtime' => $qb->createNamedParameter(1000), + 'encrypted' => $qb->createNamedParameter(0), + 'unencrypted_size' => $qb->createNamedParameter(0), + 'etag' => $qb->createNamedParameter($etag), + 'permissions' => $qb->createNamedParameter(0), + 'checksum' => $qb->createNamedParameter($etag), + ])->executeStatement(); + return $qb->getLastInsertId(); + } + + private function deleteFilecacheRow(int $fileId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) + ->executeStatement(); + } + + #[TestDox('A single run must migrate multiple different fileids in one pass, mixing both folder structures')] + public function testMigrationMultipleFileIds(): void { + $otherFileId = $this->insertFilecacheRow('test/def', 'xyz123'); + + try { + $flatFolder = $this->previewAppData->newFolder('5'); + $flatFolder->newFile('64-64-crop.jpg', 'abcdefg'); + + $hierFolder = $this->previewAppData->newFolder(self::getInternalFolder((string)$otherFileId)); + $hierFolder->newFile('128-128.png', 'abcdefg'); + + $this->invokePrivate($this->createJob(), 'run', [[]]); + + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile($otherFileId)))); + } finally { + $this->deleteFilecacheRow($otherFileId); + } + } + + #[TestDox('Re-migrating a preview that already exists must skip the insert and still clean up the stale filecache row')] + public function testMigrationSkipsDuplicatePreview(): void { + $folder = $this->previewAppData->newFolder('5'); + $folder->newFile('64-64-crop.jpg', 'abcdefg'); + + // Simulate a preview that was already migrated by an earlier, interrupted run: + // same fileid/width/height/mimetype/cropped/version as what this migration + // would produce for the file created above, so the insert below hits the + // `previews_file_uniq_idx` unique constraint. + $existing = Preview::fromPath('5/64-64-crop.jpg', $this->mimeTypeDetector); + $this->assertNotFalse($existing); + $existing->setFileId(5); + $existing->setStorageId(1); + $existing->setSourceMimeType('image/png'); + $existing->setEtag('abcdefg'); + $existing->setSize(7); + $existing->setMtime(1000); + $existing->setEncrypted(false); + $existing->generateId(); + $this->previewMapper->insert($existing); + + $this->invokePrivate($this->createJob(), 'run', [[]]); + + // No duplicate preview row was inserted, but the legacy folder and its stale + // filecache row were still cleaned up. + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(1, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + } + + #[TestDox('An orphaned preview folder whose source file no longer exists must be deleted, not migrated')] + public function testMigrationDeletesOrphanedPreview(): void { + $orphanFileId = 9999998; + $folder = $this->previewAppData->newFolder((string)$orphanFileId); + $folder->newFile('64-64-crop.jpg', 'abcdefg'); + + $this->invokePrivate($this->createJob(), 'run', [[]]); + + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile($orphanFileId)))); + } + + #[TestDox('run() must complete without error when there is nothing to migrate')] + public function testMigrationWithoutAnyPreviews(): void { + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + + $this->invokePrivate($this->createJob(), 'run', [[]]); + + $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); + $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); + } + #[TestDox("Test the migration from the 'new' nested hierarchy to the database format")] public function testMigrationPath(): void { $folder = $this->previewAppData->newFolder(self::getInternalFolder((string)5)); From 7e79fc076b7a026a4a6e72c912ad671200391024 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 13:12:56 +0200 Subject: [PATCH 5/6] refactor(Preview): Refactor tests to reduce code duplication Signed-off-by: Carl Schwan --- tests/lib/Preview/PreviewMigrationJobTest.php | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/tests/lib/Preview/PreviewMigrationJobTest.php b/tests/lib/Preview/PreviewMigrationJobTest.php index c579ce092443c..dcd57464f0d1e 100644 --- a/tests/lib/Preview/PreviewMigrationJobTest.php +++ b/tests/lib/Preview/PreviewMigrationJobTest.php @@ -116,24 +116,7 @@ public function testMigrationLegacyPath(): void { $this->assertEquals(2, count($folder->getDirectoryListing())); $this->assertEquals(0, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); - $job = new PreviewMigrationJob( - Server::get(ITimeFactory::class), - $this->appConfig, - $this->config, - Server::get(IRootFolder::class), - new PreviewMigrationService( - $this->config, - Server::get(IRootFolder::class), - $this->logger, - $this->mimeTypeDetector, - $this->mimeTypeLoader, - Server::get(IDBConnection::class), - $this->previewMapper, - $this->storageFactory, - Server::get(IAppDataFactory::class), - ), - $this->logger, - ); + $job = $this->createJob(); $this->invokePrivate($job, 'run', [[]]); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); $this->assertEquals(2, count(iterator_to_array($this->previewMapper->getAvailablePreviewsForFile(5)))); From 64cdaba77c30f01433eb775378a9c729feb0c5b5 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 28 Jul 2026 13:43:07 +0200 Subject: [PATCH 6/6] fix(Preview): Fix sharding of the PreviewMigrationService Signed-off-by: Carl Schwan --- lib/private/Preview/PreviewMigrationService.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Preview/PreviewMigrationService.php b/lib/private/Preview/PreviewMigrationService.php index 7be331362c441..377950da84952 100644 --- a/lib/private/Preview/PreviewMigrationService.php +++ b/lib/private/Preview/PreviewMigrationService.php @@ -224,6 +224,7 @@ private function folderHasChildren(int $storageId, string $path): bool { $qb->select('fileid') ->from('filecache') ->where($qb->expr()->eq('parent', $qb->createNamedParameter((int)$folderId))) + ->andWhere($qb->expr()->eq('storage', $qb->createNamedParameter($storageId))) ->setMaxResults(1); $cursor = $qb->executeQuery(); $hasChild = $cursor->fetchOne() !== false;