From 66f50bd585f428862849db0db2847287e59453c9 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 30 Sep 2025 13:44:34 +0200 Subject: [PATCH] refactor(preview): Use same mimetype ids as filecache Signed-off-by: Carl Schwan --- core/BackgroundJobs/MovePreviewJob.php | 13 ++- core/Command/Preview/ResetRenderedTexts.php | 2 +- lib/private/Files/Cache/LocalRootScanner.php | 14 +++ lib/private/Files/Cache/Scanner.php | 22 ++-- lib/private/Preview/Db/Preview.php | 73 ++++++------ lib/private/Preview/Db/PreviewMapper.php | 22 +--- lib/private/Preview/Generator.php | 106 ++++++------------ .../Preview/Storage/LocalPreviewStorage.php | 28 ++++- lib/private/Preview/Storage/PreviewFile.php | 8 +- .../Preview/Storage/StorageFactory.php | 6 +- lib/public/IPreview.php | 5 - tests/lib/Preview/GeneratorTest.php | 39 ++++--- tests/lib/Preview/MovePreviewJobTest.php | 23 +++- tests/lib/Preview/PreviewMapperTest.php | 5 +- tests/lib/Preview/PreviewServiceTest.php | 2 +- 15 files changed, 186 insertions(+), 182 deletions(-) diff --git a/core/BackgroundJobs/MovePreviewJob.php b/core/BackgroundJobs/MovePreviewJob.php index 0eaf409e433..ff13a6f37e2 100644 --- a/core/BackgroundJobs/MovePreviewJob.php +++ b/core/BackgroundJobs/MovePreviewJob.php @@ -19,12 +19,15 @@ use OCP\BackgroundJob\TimedJob; use OCP\DB\Exception; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\IAppData; +use OCP\Files\IMimeTypeDetector; +use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IAppConfig; use OCP\IDBConnection; use Override; +use Psr\Log\LoggerInterface; class MovePreviewJob extends TimedJob { private IAppData $appData; @@ -36,6 +39,9 @@ class MovePreviewJob extends TimedJob { private readonly StorageFactory $storageFactory, private readonly IDBConnection $connection, private readonly IRootFolder $rootFolder, + private readonly IMimeTypeDetector $mimeTypeDetector, + private readonly IMimeTypeLoader $mimeTypeLoader, + private readonly LoggerInterface $logger, IAppDataFactory $appDataFactory, ) { parent::__construct($time); @@ -125,8 +131,13 @@ class MovePreviewJob extends TimedJob { $previewFiles = []; foreach ($folder->getDirectoryListing() as $previewFile) { + $path = $fileId . '/' . $previewFile->getName(); /** @var SimpleFile $previewFile */ - $preview = Preview::fromPath($fileId . '/' . $previewFile->getName()); + $preview = Preview::fromPath($path, $this->mimeTypeDetector, $this->mimeTypeLoader); + if (!$preview) { + $this->logger->error('Unable to import old preview at path.'); + continue; + } $preview->setSize($previewFile->getSize()); $preview->setMtime($previewFile->getMtime()); $preview->setOldFileId($previewFile->getId()); diff --git a/core/Command/Preview/ResetRenderedTexts.php b/core/Command/Preview/ResetRenderedTexts.php index 6ec26ad6ac3..c4fcabbe500 100644 --- a/core/Command/Preview/ResetRenderedTexts.php +++ b/core/Command/Preview/ResetRenderedTexts.php @@ -93,7 +93,7 @@ class ResetRenderedTexts extends Command { $previewsToDeleteCount = 0; foreach ($this->getPreviewsToDelete() as $preview) { - $output->writeln('Deleting preview ' . $preview->getName() . ' for fileId ' . $preview->getFileId(), OutputInterface::VERBOSITY_VERBOSE); + $output->writeln('Deleting preview ' . $preview->getName($this->mimeTypeLoader) . ' for fileId ' . $preview->getFileId(), OutputInterface::VERBOSITY_VERBOSE); $previewsToDeleteCount++; diff --git a/lib/private/Files/Cache/LocalRootScanner.php b/lib/private/Files/Cache/LocalRootScanner.php index 3f4f70b865b..79908d63fe2 100644 --- a/lib/private/Files/Cache/LocalRootScanner.php +++ b/lib/private/Files/Cache/LocalRootScanner.php @@ -8,7 +8,18 @@ declare(strict_types=1); */ namespace OC\Files\Cache; +use OCP\IConfig; +use OCP\Server; + class LocalRootScanner extends Scanner { + private string $previewFolder; + + public function __construct(\OC\Files\Storage\Storage $storage) { + parent::__construct($storage); + $config = Server::get(IConfig::class); + $this->previewFolder = 'appdata_' . $config->getSystemValueString('instanceid', '') . '/preview'; + } + public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) { if ($this->shouldScanPath($file)) { return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock, $data); @@ -27,6 +38,9 @@ class LocalRootScanner extends Scanner { private function shouldScanPath(string $path): bool { $path = trim($path, '/'); + if (str_starts_with($path, $this->previewFolder)) { + return false; + } return $path === '' || str_starts_with($path, 'appdata_') || str_starts_with($path, '__groupfolders'); } } diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 75ca9da0abe..2fa0dd09e4f 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -11,14 +11,15 @@ use Doctrine\DBAL\Exception; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\Storage\Wrapper\Jail; use OC\Hooks\BasicEmitter; -use OC\SystemConfig; use OCP\Files\Cache\IScanner; use OCP\Files\ForbiddenException; use OCP\Files\NotFoundException; use OCP\Files\Storage\ILockingStorage; use OCP\Files\Storage\IReliableEtagStorage; +use OCP\IConfig; use OCP\IDBConnection; use OCP\Lock\ILockingProvider; +use OCP\Server; use Psr\Log\LoggerInterface; /** @@ -65,19 +66,15 @@ class Scanner extends BasicEmitter implements IScanner { protected IDBConnection $connection; - private string $previewFolder; - public function __construct(\OC\Files\Storage\Storage $storage) { $this->storage = $storage; $this->storageId = $this->storage->getId(); $this->cache = $storage->getCache(); - /** @var SystemConfig $config */ - $config = \OC::$server->get(SystemConfig::class); - $this->cacheActive = !$config->getValue('filesystem_cache_readonly', false); - $this->useTransactions = !$config->getValue('filescanner_no_transactions', false); - $this->lockingProvider = \OC::$server->get(ILockingProvider::class); - $this->connection = \OC::$server->get(IDBConnection::class); - $this->previewFolder = 'appdata_' . $config->getValue('instanceid', '') . '/preview'; + $config = Server::get(IConfig::class); + $this->cacheActive = !$config->getSystemValueBool('filesystem_cache_readonly', false); + $this->useTransactions = !$config->getSystemValueBool('filescanner_no_transactions', false); + $this->lockingProvider = Server::get(ILockingProvider::class); + $this->connection = Server::get(IDBConnection::class); } /** @@ -415,11 +412,6 @@ class Scanner extends BasicEmitter implements IScanner { $size = 0; $childQueue = $this->handleChildren($path, $recursive, $reuse, $folderId, $lock, $size, $etagChanged); - if (str_starts_with($path, $this->previewFolder)) { - // Preview scanning is handled in LocalPreviewStorage - return 0; - } - foreach ($childQueue as $child => [$childId, $childSize]) { // "etag changed" propagates up, but not down, so we pass `false` to the children even if we already know that the etag of the current folder changed $childEtagChanged = false; diff --git a/lib/private/Preview/Db/Preview.php b/lib/private/Preview/Db/Preview.php index fe30da6589c..7eb03e1e289 100644 --- a/lib/private/Preview/Db/Preview.php +++ b/lib/private/Preview/Db/Preview.php @@ -12,7 +12,9 @@ namespace OC\Preview\Db; use OCP\AppFramework\Db\Entity; use OCP\DB\Types; -use OCP\IPreview; +use OCP\Files\IMimeTypeDetector; +use OCP\Files\IMimeTypeLoader; +use OCP\Server; /** * Preview entity mapped to the oc_previews and oc_preview_locations table. @@ -91,42 +93,39 @@ class Preview extends Entity { $this->addType('version', Types::BIGINT); } - public static function fromPath(string $path): Preview { + public static function fromPath(string $path, IMimeTypeDetector $mimeTypeDetector, IMimeTypeLoader $mimeTypeLoader): Preview|false { $preview = new self(); $preview->setFileId((int)basename(dirname($path))); $fileName = pathinfo($path, PATHINFO_FILENAME) . '.' . pathinfo($path, PATHINFO_EXTENSION); + $ok = preg_match('/(([0-9]+)-)?([0-9]+)-([0-9]+)(-(max))?(-(crop))?\.([a-z]{3,4})/', $fileName, $matches); - [0 => $baseName, 1 => $extension] = explode('.', $fileName); - $preview->setMimetype(match ($extension) { - 'jpg' | 'jpeg' => IPreview::MIMETYPE_JPEG, - 'png' => IPreview::MIMETYPE_PNG, - 'gif' => IPreview::MIMETYPE_GIF, - 'webp' => IPreview::MIMETYPE_WEBP, - default => IPreview::MIMETYPE_JPEG, - }); - $nameSplit = explode('-', $baseName); - - $offset = 0; - $preview->setVersion(null); - if (count($nameSplit) === 4 || (count($nameSplit) === 3 && is_numeric($nameSplit[2]))) { - $offset = 1; - $preview->setVersion((int)$nameSplit[0]); + if ($ok !== 1) { + return false; } - $preview->setWidth((int)$nameSplit[$offset + 0]); - $preview->setHeight((int)$nameSplit[$offset + 1]); + [ + 2 => $version, + 3 => $width, + 4 => $height, + 6 => $crop, + 8 => $max, + ] = $matches; - $preview->setCropped(false); - $preview->setMax(false); - if (isset($nameSplit[$offset + 2])) { - $preview->setCropped($nameSplit[$offset + 2] === 'crop'); - $preview->setMax($nameSplit[$offset + 2] === 'max'); + $preview->setMimetype($mimeTypeLoader->getId($mimeTypeDetector->detectPath($fileName))); + + $preview->setWidth((int)$width); + $preview->setHeight((int)$height); + $preview->setCropped($crop === 'crop'); + $preview->setMax($max === 'max'); + + if (!empty($version)) { + $preview->setVersion((int)$version); } return $preview; } - public function getName(): string { + public function getName(IMimeTypeLoader $mimeTypeLoader): string { $path = ($this->getVersion() > -1 ? $this->getVersion() . '-' : '') . $this->getWidth() . '-' . $this->getHeight(); if ($this->isCropped()) { $path .= '-crop'; @@ -135,27 +134,23 @@ class Preview extends Entity { $path .= '-max'; } - $ext = $this->getExtension(); + $ext = $this->getExtension($mimeTypeLoader); $path .= '.' . $ext; return $path; } - public function getMimetypeValue(): string { - return match ($this->mimetype) { - IPreview::MIMETYPE_JPEG => 'image/jpeg', - IPreview::MIMETYPE_PNG => 'image/png', - IPreview::MIMETYPE_WEBP => 'image/webp', - IPreview::MIMETYPE_GIF => 'image/gif', + public function getExtension(IMimeTypeLoader $mimeTypeLoader): string { + return match ($this->getMimetypeValue($mimeTypeLoader)) { + 'image/png' => 'png', + 'image/gif' => 'gif', + 'image/jpeg' => 'jpg', + 'image/webp' => 'webp', + default => 'png', }; } - public function getExtension(): string { - return match ($this->mimetype) { - IPreview::MIMETYPE_JPEG => 'jpg', - IPreview::MIMETYPE_PNG => 'png', - IPreview::MIMETYPE_WEBP => 'webp', - IPreview::MIMETYPE_GIF => 'gif', - }; + public function getMimetypeValue(IMimeTypeLoader $mimeTypeLoader): string { + return $mimeTypeLoader->getMimetypeById($this->mimetype) ?? 'image/jpeg'; } public function setBucketName(string $bucketName): void { diff --git a/lib/private/Preview/Db/PreviewMapper.php b/lib/private/Preview/Db/PreviewMapper.php index d4ef242eb21..7d399dc5f3d 100644 --- a/lib/private/Preview/Db/PreviewMapper.php +++ b/lib/private/Preview/Db/PreviewMapper.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; use OCP\IPreview; @@ -24,7 +25,9 @@ class PreviewMapper extends QBMapper { private const TABLE_NAME = 'previews'; private const LOCATION_TABLE_NAME = 'preview_locations'; - public function __construct(IDBConnection $db) { + public function __construct( + IDBConnection $db, + ) { parent::__construct($db, self::TABLE_NAME, Preview::class); } @@ -57,23 +60,6 @@ class PreviewMapper extends QBMapper { return $previews; } - public function getPreview(int $fileId, int $width, int $height, string $mode, int $mimetype = IPreview::MIMETYPE_JPEG): ?Preview { - $selectQb = $this->db->getQueryBuilder(); - $this->joinLocation($selectQb) - ->where( - $selectQb->expr()->eq('file_id', $selectQb->createNamedParameter($fileId)), - $selectQb->expr()->eq('width', $selectQb->createNamedParameter($width)), - $selectQb->expr()->eq('height', $selectQb->createNamedParameter($height)), - $selectQb->expr()->eq('mode', $selectQb->createNamedParameter($mode)), - $selectQb->expr()->eq('mimetype', $selectQb->createNamedParameter($mimetype)), - ); - try { - return $this->findEntity($selectQb); - } catch (DoesNotExistException) { - return null; - } - } - /** * @return \Generator */ diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 1743ca97e0d..664448b7d01 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -152,7 +152,7 @@ class Generator { // No need to generate a preview that is just the max preview if ($width === $maxWidth && $height === $maxHeight) { // ensure correct return value if this was the last one - $previewFile = new PreviewFile($maxPreview, $this->storageFactory, $this->previewMapper); + $previewFile = new PreviewFile($maxPreview, $this->storageFactory, $this->previewMapper, $this->mimeTypeLoader); continue; } @@ -163,14 +163,14 @@ class Generator { && $preview->getVersion() === $previewVersion && $preview->isCropped() === $crop); if ($preview) { - $previewFile = new PreviewFile($preview, $this->storageFactory, $this->previewMapper); + $previewFile = new PreviewFile($preview, $this->storageFactory, $this->previewMapper, $this->mimeTypeLoader); } else { if (!$this->previewManager->isMimeSupported($mimeType)) { throw new NotFoundException(); } if ($maxPreviewImage === null) { - $maxPreviewImage = $this->helper->getImage(new PreviewFile($maxPreview, $this->storageFactory, $this->previewMapper)); + $maxPreviewImage = $this->helper->getImage(new PreviewFile($maxPreview, $this->storageFactory, $this->previewMapper, $this->mimeTypeLoader)); } $this->logger->debug('Cached preview not found for file {path}, generating a new preview.', ['path' => $file->getPath()]); @@ -350,7 +350,21 @@ class Generator { } try { - return $this->savePreview($file, $preview->width(), $preview->height(), $crop, $max, $preview, $version); + $previewEntry = new Preview(); + $previewEntry->setFileId($file->getId()); + $previewEntry->setStorageId($file->getMountPoint()->getNumericStorageId()); + $previewEntry->setSourceMimetype($this->mimeTypeLoader->getId($file->getMimeType())); + $previewEntry->setWidth($preview->width()); + $previewEntry->setHeight($preview->height()); + $previewEntry->setVersion($version); + $previewEntry->setMax($max); + $previewEntry->setCropped($crop); + $previewEntry->setEncrypted(false); + $previewEntry->setMimetype($this->mimeTypeLoader->getId($preview->dataMimeType())); + $previewEntry->setEtag($file->getEtag()); + $previewEntry->setMtime((new \DateTime())->getTimestamp()); + $previewEntry->setSize(0); + return $this->savePreview($previewEntry, $preview); } catch (NotPermittedException) { throw new NotFoundException(); } @@ -360,21 +374,6 @@ class Generator { throw new NotFoundException('No provider successfully handled the preview generation'); } - private function generatePath(int $width, int $height, bool $crop, bool $max, string $mimeType, int $version): string { - $path = ($version !== -1 ? $version . '-' : '') . $width . '-' . $height; - if ($crop) { - $path .= '-crop'; - } - if ($max) { - $path .= '-max'; - } - - $ext = $this->getExtension($mimeType); - $path .= '.' . $ext; - return $path; - } - - /** * @psalm-param IPreview::MODE_* $mode * @return int[] @@ -505,30 +504,25 @@ class Generator { self::unguardWithSemaphore($sem); } - $path = $this->generatePath($width, $height, $crop, false, $preview->dataMimeType(), $version); + $previewEntry = new Preview(); + $previewEntry->setFileId($file->getId()); + $previewEntry->setStorageId($file->getMountPoint()->getNumericStorageId()); + $previewEntry->setWidth($width); + $previewEntry->setSourceMimetype($this->mimeTypeLoader->getId($file->getMimeType())); + $previewEntry->setHeight($height); + $previewEntry->setVersion($version); + $previewEntry->setMax(false); + $previewEntry->setCropped($crop); + $previewEntry->setEncrypted(false); + $previewEntry->setMimetype($this->mimeTypeLoader->getId($preview->dataMimeType())); + $previewEntry->setEtag($file->getEtag()); + $previewEntry->setMtime((new \DateTime())->getTimestamp()); + $previewEntry->setSize(0); if ($cacheResult) { - $previewEntry = $this->savePreview($file, $width, $height, $crop, false, $preview, $version); - return new PreviewFile($previewEntry, $this->storageFactory, $this->previewMapper); + $previewEntry = $this->savePreview($previewEntry, $preview); + return new PreviewFile($previewEntry, $this->storageFactory, $this->previewMapper, $this->mimeTypeLoader); } else { - return new InMemoryFile($path, $preview->data()); - } - } - - /** - * @throws \InvalidArgumentException - */ - private function getExtension(string $mimeType): string { - switch ($mimeType) { - case 'image/png': - return 'png'; - case 'image/jpeg': - return 'jpg'; - case 'image/webp': - return 'webp'; - case 'image/gif': - return 'gif'; - default: - throw new \InvalidArgumentException('Not a valid mimetype: "' . $mimeType . '"'); + return new InMemoryFile($previewEntry->getName($this->mimeTypeLoader), $preview->data()); } } @@ -538,35 +532,7 @@ class Generator { * @throws NotPermittedException * @throws \OCP\DB\Exception */ - public function savePreview(File $file, int $width, int $height, bool $crop, bool $max, IImage $preview, int $version): Preview { - $previewEntry = new Preview(); - $previewEntry->setFileId($file->getId()); - $previewEntry->setStorageId($file->getMountPoint()->getNumericStorageId()); - $previewEntry->setWidth($width); - $previewEntry->setSourceMimetype($this->mimeTypeLoader->getId($file->getMimeType())); - $previewEntry->setHeight($height); - $previewEntry->setVersion($version); - $previewEntry->setMax($max); - $previewEntry->setCropped($crop); - $previewEntry->setEncrypted(false); - switch ($preview->dataMimeType()) { - case 'image/jpeg': - $previewEntry->setMimetype(IPreview::MIMETYPE_JPEG); - break; - case 'image/gif': - $previewEntry->setMimetype(IPreview::MIMETYPE_GIF); - break; - case 'image/webp': - $previewEntry->setMimetype(IPreview::MIMETYPE_WEBP); - break; - default: - $previewEntry->setMimetype(IPreview::MIMETYPE_PNG); - break; - } - $previewEntry->setEtag($file->getEtag()); - $previewEntry->setMtime((new \DateTime())->getTimestamp()); - $previewEntry->setSize(0); - + public function savePreview(Preview $previewEntry, IImage $preview): Preview { $previewEntry = $this->previewMapper->insert($previewEntry); // we need to save to DB first diff --git a/lib/private/Preview/Storage/LocalPreviewStorage.php b/lib/private/Preview/Storage/LocalPreviewStorage.php index 8d85897115b..e522df7da8e 100644 --- a/lib/private/Preview/Storage/LocalPreviewStorage.php +++ b/lib/private/Preview/Storage/LocalPreviewStorage.php @@ -16,10 +16,13 @@ use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; use OCP\DB\Exception; +use OCP\Files\IMimeTypeDetector; +use OCP\Files\IMimeTypeLoader; use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use Override; +use Psr\Log\LoggerInterface; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -33,6 +36,9 @@ class LocalPreviewStorage implements IPreviewStorage { private readonly StorageFactory $previewStorage, private readonly IAppConfig $appConfig, private readonly IDBConnection $connection, + private readonly IMimeTypeLoader $mimeTypeLoader, + private readonly IMimeTypeDetector $mimeTypeDetector, + private readonly LoggerInterface $logger, ) { $this->instanceId = $this->config->getSystemValueString('instanceid'); $this->rootFolder = $this->config->getSystemValue('datadirectory', OC::$SERVERROOT . '/data'); @@ -62,7 +68,7 @@ class LocalPreviewStorage implements IPreviewStorage { } private function constructPath(Preview $preview): string { - return $this->getPreviewRootFolder() . implode('/', str_split(substr(md5((string)$preview->getFileId()), 0, 7))) . '/' . $preview->getFileId() . '/' . $preview->getName(); + return $this->getPreviewRootFolder() . implode('/', str_split(substr(md5((string)$preview->getFileId()), 0, 7))) . '/' . $preview->getFileId() . '/' . $preview->getName($this->mimeTypeLoader); } private function createParentFiles(string $path): bool { @@ -74,7 +80,7 @@ class LocalPreviewStorage implements IPreviewStorage { #[Override] public function migratePreview(Preview $preview, SimpleFile $file): void { // legacy flat directory - $sourcePath = $this->getPreviewRootFolder() . $preview->getFileId() . '/' . $preview->getName(); + $sourcePath = $this->getPreviewRootFolder() . $preview->getFileId() . '/' . $preview->getName($this->mimeTypeLoader); if (!file_exists($sourcePath)) { return; } @@ -88,7 +94,7 @@ class LocalPreviewStorage implements IPreviewStorage { $this->createParentFiles($destinationPath); $ok = rename($sourcePath, $destinationPath); if (!$ok) { - throw new LogicException('Failed to copy ' . $sourcePath . ' to ' . $destinationPath); + throw new LogicException('Failed to move ' . $sourcePath . ' to ' . $destinationPath); } } @@ -100,7 +106,11 @@ class LocalPreviewStorage implements IPreviewStorage { $previewsFound = 0; foreach (new RecursiveIteratorIterator($scanner) as $file) { if ($file->isFile()) { - $preview = Preview::fromPath((string)$file); + $preview = Preview::fromPath((string)$file, $this->mimeTypeDetector, $this->mimeTypeLoader); + if ($preview === false) { + $this->logger->error('Unable to parse preview information for ' . $file->getRealPath()); + continue; + } try { $preview->setSize($file->getSize()); $preview->setMtime($file->getMtime()); @@ -139,7 +149,15 @@ class LocalPreviewStorage implements IPreviewStorage { $this->previewMapper->insert($preview); // Move old flat preview to new format - $this->previewStorage->migratePreview($preview, $file); + $dirName = str_replace($this->getPreviewRootFolder(), '', $file->getPath()); + if (preg_match('/[0-9a-e]\/[0-9a-e]\/[0-9a-e]\/[0-9a-e]\/[0-9a-e]\/[0-9a-e]\/[0-9a-e]\/[0-9]+/', $dirName) !== 1) { + $previewPath = $this->constructPath($preview); + $this->createParentFiles($previewPath); + $ok = rename($file->getRealPath(), $previewPath); + if (!$ok) { + throw new LogicException('Failed to move ' . $file->getRealPath() . ' to ' . $previewPath); + } + } } catch (Exception $e) { if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { throw $e; diff --git a/lib/private/Preview/Storage/PreviewFile.php b/lib/private/Preview/Storage/PreviewFile.php index 8c9852673cc..c7c7b59c97c 100644 --- a/lib/private/Preview/Storage/PreviewFile.php +++ b/lib/private/Preview/Storage/PreviewFile.php @@ -12,6 +12,7 @@ namespace OC\Preview\Storage; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; +use OCP\Files\IMimeTypeLoader; use OCP\Files\SimpleFS\ISimpleFile; use Override; @@ -20,12 +21,13 @@ class PreviewFile implements ISimpleFile { private readonly Preview $preview, private readonly IPreviewStorage $storage, private readonly PreviewMapper $previewMapper, + private readonly IMimeTypeLoader $mimeTypeLoader, ) { } #[Override] public function getName(): string { - return $this->preview->getName(); + return $this->preview->getName($this->mimeTypeLoader); } #[Override] @@ -61,12 +63,12 @@ class PreviewFile implements ISimpleFile { #[Override] public function getMimeType(): string { - return $this->preview->getMimetypeValue(); + return $this->preview->getMimetypeValue($this->mimeTypeLoader); } #[Override] public function getExtension(): string { - return $this->preview->getExtension(); + return $this->preview->getExtension($this->mimeTypeLoader); } #[Override] diff --git a/lib/private/Preview/Storage/StorageFactory.php b/lib/private/Preview/Storage/StorageFactory.php index 5d1d130b300..e33135be3ce 100644 --- a/lib/private/Preview/Storage/StorageFactory.php +++ b/lib/private/Preview/Storage/StorageFactory.php @@ -13,8 +13,6 @@ namespace OC\Preview\Storage; use OC\Files\ObjectStore\PrimaryObjectStoreConfig; use OC\Files\SimpleFS\SimpleFile; use OC\Preview\Db\Preview; -use OC\Preview\Db\PreviewMapper; -use OCP\IConfig; use OCP\Server; use Override; @@ -23,8 +21,6 @@ class StorageFactory implements IPreviewStorage { public function __construct( private readonly PrimaryObjectStoreConfig $objectStoreConfig, - private readonly IConfig $config, - private readonly PreviewMapper $previewMapper, ) { } @@ -49,7 +45,7 @@ class StorageFactory implements IPreviewStorage { } if ($this->objectStoreConfig->hasObjectStore()) { - $this->backend = new ObjectStorePreviewStorage($this->objectStoreConfig, $this->config, $this->previewMapper); + $this->backend = Server::get(ObjectStorePreviewStorage::class); } else { $this->backend = Server::get(LocalPreviewStorage::class); } diff --git a/lib/public/IPreview.php b/lib/public/IPreview.php index cbd0e0ae525..3c9eadd4577 100644 --- a/lib/public/IPreview.php +++ b/lib/public/IPreview.php @@ -29,11 +29,6 @@ interface IPreview { */ public const MODE_COVER = 'cover'; - public const MIMETYPE_JPEG = 0; - public const MIMETYPE_WEBP = 1; - public const MIMETYPE_PNG = 2; - public const MIMETYPE_GIF = 3; - /** * In order to improve lazy loading a closure can be registered which will be * called in case preview providers are actually requested diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php index 640c2ce8b36..fc48ebc181a 100644 --- a/tests/lib/Preview/GeneratorTest.php +++ b/tests/lib/Preview/GeneratorTest.php @@ -47,7 +47,12 @@ class GeneratorTest extends TestCase { $this->logger = $this->createMock(LoggerInterface::class); $this->previewMapper = $this->createMock(PreviewMapper::class); $this->storageFactory = $this->createMock(StorageFactory::class); - $this->mimetypeLoader = $this->createMock(IMimeTypeLoader::class); + $this->mimeTypeLoader = $this->createMock(IMimeTypeLoader::class); + $this->mimeTypeLoader->method('getId') + ->willReturnCallback(fn ($mimeType) => $mimeType === 'image/png' ? 42 : 43); + $this->mimeTypeLoader->method('getMimetypeById') + ->with(42) + ->willReturn('image/png'); $this->generator = new Generator( $this->config, @@ -57,7 +62,7 @@ class GeneratorTest extends TestCase { $this->logger, $this->previewMapper, $this->storageFactory, - $this->mimetypeLoader, + $this->mimeTypeLoader, ); } @@ -91,7 +96,7 @@ class GeneratorTest extends TestCase { $maxPreview->setVersion(-1); $maxPreview->setCropped(false); $maxPreview->setStorageId(1); - $maxPreview->setMimetype(IPreview::MIMETYPE_PNG); + $maxPreview->setMimetype($this->mimeTypeLoader->getId('image/png')); $previewFile = new Preview(); $previewFile->setWidth(256); @@ -101,7 +106,7 @@ class GeneratorTest extends TestCase { $previewFile->setVersion(-1); $previewFile->setCropped(false); $previewFile->setStorageId(1); - $previewFile->setMimetype(IPreview::MIMETYPE_PNG); + $previewFile->setMimetype($this->mimeTypeLoader->getId('image/png')); $this->previewMapper->method('getAvailablePreviews') ->with($this->equalTo([42])) @@ -130,7 +135,7 @@ class GeneratorTest extends TestCase { ->with($this->equalTo([42])) ->willReturn([42 => []]); - $this->config->method('getSystemValue') + $this->config->method('getSystemValueString') ->willReturnCallback(function ($key, $default) { return $default; }); @@ -196,7 +201,7 @@ class GeneratorTest extends TestCase { $maxPreview->setHeight(2048); $maxPreview->setMax(true); $maxPreview->setSize(1000); - $maxPreview->setMimetype(IPreview::MIMETYPE_PNG); + $maxPreview->setMimetype($this->mimeTypeLoader->getId('image/png')); $this->previewMapper->method('insert') ->willReturnCallback(fn (Preview $preview): Preview => $preview); @@ -206,7 +211,7 @@ class GeneratorTest extends TestCase { $this->storageFactory->method('writePreview') ->willReturnCallback(function (Preview $preview, string $data): int { - switch ($preview->getName()) { + switch ($preview->getName($this->mimeTypeLoader)) { case '2048-2048-max.png': $this->assertSame('my data', $data); return 1000; @@ -214,7 +219,7 @@ class GeneratorTest extends TestCase { $this->assertSame('my resized data', $data); return 1000; } - $this->fail('file name is wrong:' . $preview->getName()); + $this->fail('file name is wrong:' . $preview->getName($this->mimeTypeLoader)); }); $image = $this->getMockImage(2048, 2048, 'my resized data'); @@ -245,7 +250,7 @@ class GeneratorTest extends TestCase { $maxPreview->setMax(true); $maxPreview->setSize(1000); $maxPreview->setVersion(-1); - $maxPreview->setMimetype(IPreview::MIMETYPE_PNG); + $maxPreview->setMimetype(42); $this->previewMapper->method('getAvailablePreviews') ->with($this->equalTo([42])) @@ -269,7 +274,7 @@ class GeneratorTest extends TestCase { $maxPreview->setMax(true); $maxPreview->setSize(1000); $maxPreview->setVersion(-1); - $maxPreview->setMimetype(IPreview::MIMETYPE_PNG); + $maxPreview->setMimetype($this->mimeTypeLoader->getId('image/png')); $previewFile = new Preview(); $previewFile->setWidth(1024); @@ -278,7 +283,7 @@ class GeneratorTest extends TestCase { $previewFile->setSize(1000); $previewFile->setCropped(true); $previewFile->setVersion(-1); - $previewFile->setMimetype(IPreview::MIMETYPE_PNG); + $previewFile->setMimetype($this->mimeTypeLoader->getId('image/png')); $this->previewMapper->method('getAvailablePreviews') ->with($this->equalTo([42])) @@ -316,7 +321,7 @@ class GeneratorTest extends TestCase { $this->generator->getPreview($file, 100, 100); } - private function getMockImage($width, $height, $data = null) { + private function getMockImage(int $width, int $height, $data = null) { $image = $this->createMock(IImage::class); $image->method('height')->willReturn($width); $image->method('width')->willReturn($height); @@ -387,10 +392,10 @@ class GeneratorTest extends TestCase { $maxPreview->setMax(true); $maxPreview->setSize(1000); $maxPreview->setVersion(-1); - $maxPreview->setMimetype(IPreview::MIMETYPE_PNG); + $maxPreview->setMimetype(42); - $this->assertSame($maxPreview->getName(), $maxX . '-' . $maxY . '-max.png'); - $this->assertSame($maxPreview->getMimetypeValue(), 'image/png'); + $this->assertSame($maxPreview->getName($this->mimeTypeLoader), $maxX . '-' . $maxY . '-max.png'); + $this->assertSame($maxPreview->getMimetypeValue($this->mimeTypeLoader), 'image/png'); $this->previewMapper->method('getAvailablePreviews') ->with($this->equalTo([42])) @@ -410,7 +415,7 @@ class GeneratorTest extends TestCase { $this->previewMapper->method('insert') ->willReturnCallback(function (Preview $preview) use ($filename): Preview { - $this->assertSame($preview->getName(), $filename); + $this->assertSame($preview->getName($this->mimeTypeLoader), $filename); return $preview; }); @@ -426,7 +431,7 @@ class GeneratorTest extends TestCase { $result = $this->generator->getPreview($file, $reqX, $reqY, $crop, $mode); if ($expectedX === $maxX && $expectedY === $maxY) { - $this->assertSame($maxPreview->getName(), $result->getName()); + $this->assertSame($maxPreview->getName($this->mimeTypeLoader), $result->getName()); } else { $this->assertSame($filename, $result->getName()); } diff --git a/tests/lib/Preview/MovePreviewJobTest.php b/tests/lib/Preview/MovePreviewJobTest.php index cd9f3253654..a5ac5ad51e3 100644 --- a/tests/lib/Preview/MovePreviewJobTest.php +++ b/tests/lib/Preview/MovePreviewJobTest.php @@ -17,12 +17,15 @@ use OC\Preview\Storage\StorageFactory; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\IAppData; +use OCP\Files\IMimeTypeDetector; +use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; use OCP\IAppConfig; use OCP\IDBConnection; use OCP\Server; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; use Test\TestCase; /** @@ -35,6 +38,9 @@ class MovePreviewJobTest extends TestCase { private StorageFactory $storageFactory; private PreviewService $previewService; private IDBConnection $db; + private IMimeTypeLoader&MockObject $mimeTypeLoader; + private IMimeTypeDetector&MockObject $mimeTypeDetector; + private LoggerInterface&MockObject $logger; public function setUp(): void { parent::setUp(); @@ -75,6 +81,12 @@ class MovePreviewJobTest extends TestCase { 'permissions' => $qb->createNamedParameter(0), 'checksum' => $qb->createNamedParameter('abcdefg'), ])->executeStatement(); + + $this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class); + $this->mimeTypeDetector->method('detectPath')->willReturn('image/png'); + $this->mimeTypeLoader = $this->createMock(IMimeTypeLoader::class); + $this->mimeTypeLoader->method('getId')->with('image/png')->willReturn(42); + $this->logger = $this->createMock(LoggerInterface::class); } public function tearDown(): void { @@ -105,7 +117,10 @@ class MovePreviewJobTest extends TestCase { $this->storageFactory, Server::get(IDBConnection::class), Server::get(IRootFolder::class), - Server::get(IAppDataFactory::class) + $this->mimeTypeDetector, + $this->mimeTypeLoader, + $this->logger, + Server::get(IAppDataFactory::class), ); $this->invokePrivate($job, 'run', [[]]); $this->assertEquals(0, count($this->previewAppData->getDirectoryListing())); @@ -133,6 +148,9 @@ class MovePreviewJobTest extends TestCase { $this->storageFactory, Server::get(IDBConnection::class), Server::get(IRootFolder::class), + $this->mimeTypeDetector, + $this->mimeTypeLoader, + $this->logger, Server::get(IAppDataFactory::class) ); $this->invokePrivate($job, 'run', [[]]); @@ -169,6 +187,9 @@ class MovePreviewJobTest extends TestCase { $this->storageFactory, Server::get(IDBConnection::class), Server::get(IRootFolder::class), + $this->mimeTypeDetector, + $this->mimeTypeLoader, + $this->logger, Server::get(IAppDataFactory::class) ); $this->invokePrivate($job, 'run', [[]]); diff --git a/tests/lib/Preview/PreviewMapperTest.php b/tests/lib/Preview/PreviewMapperTest.php index e4b2b1ab21a..19018c8b318 100644 --- a/tests/lib/Preview/PreviewMapperTest.php +++ b/tests/lib/Preview/PreviewMapperTest.php @@ -12,6 +12,7 @@ namespace Test\Preview; use OC\Preview\Db\Preview; use OC\Preview\Db\PreviewMapper; +use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; use OCP\IPreview; use OCP\Server; @@ -23,10 +24,12 @@ use Test\TestCase; class PreviewMapperTest extends TestCase { private PreviewMapper $previewMapper; private IDBConnection $connection; + private IMimeTypeLoader $mimeTypeLoader; public function setUp(): void { $this->previewMapper = Server::get(PreviewMapper::class); $this->connection = Server::get(IDBConnection::class); + $this->mimeTypeLoader = Server::get(IMimeTypeLoader::class); } public function testGetAvailablePreviews(): void { @@ -72,7 +75,7 @@ class PreviewMapperTest extends TestCase { $preview->setHeight(100); $preview->setSize(100); $preview->setMtime(time()); - $preview->setMimetype(IPreview::MIMETYPE_PNG); + $preview->setMimetype($this->mimeTypeLoader->getId('image/jpeg')); $preview->setEtag('abcdefg'); if ($locationId !== null) { diff --git a/tests/lib/Preview/PreviewServiceTest.php b/tests/lib/Preview/PreviewServiceTest.php index f9a8d88f664..fe8dd1c3d33 100644 --- a/tests/lib/Preview/PreviewServiceTest.php +++ b/tests/lib/Preview/PreviewServiceTest.php @@ -46,7 +46,7 @@ class PreviewServiceTest extends TestCase { $preview->setSourceMimetype(1); $preview->setCropped(true); $preview->setEncrypted(false); - $preview->setMimetype(IPreview::MIMETYPE_JPEG); + $preview->setMimetype(42); $preview->setEtag('abc'); $preview->setMtime((new \DateTime())->getTimestamp()); $preview->setSize(0);