diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 848bd66185d83..db4462006f280 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2171,6 +2171,7 @@ 'OC\\Preview\\Photoshop' => $baseDir . '/lib/private/Preview/Photoshop.php', 'OC\\Preview\\Postscript' => $baseDir . '/lib/private/Preview/Postscript.php', 'OC\\Preview\\PreviewMigrationService' => $baseDir . '/lib/private/Preview/PreviewMigrationService.php', + 'OC\\Preview\\PreviewProviderDefaults' => $baseDir . '/lib/private/Preview/PreviewProviderDefaults.php', 'OC\\Preview\\PreviewService' => $baseDir . '/lib/private/Preview/PreviewService.php', 'OC\\Preview\\ProviderV2' => $baseDir . '/lib/private/Preview/ProviderV2.php', 'OC\\Preview\\SGI' => $baseDir . '/lib/private/Preview/SGI.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 787e012e3215a..492c2f7d33928 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2212,6 +2212,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Preview\\Photoshop' => __DIR__ . '/../../..' . '/lib/private/Preview/Photoshop.php', 'OC\\Preview\\Postscript' => __DIR__ . '/../../..' . '/lib/private/Preview/Postscript.php', 'OC\\Preview\\PreviewMigrationService' => __DIR__ . '/../../..' . '/lib/private/Preview/PreviewMigrationService.php', + 'OC\\Preview\\PreviewProviderDefaults' => __DIR__ . '/../../..' . '/lib/private/Preview/PreviewProviderDefaults.php', 'OC\\Preview\\PreviewService' => __DIR__ . '/../../..' . '/lib/private/Preview/PreviewService.php', 'OC\\Preview\\ProviderV2' => __DIR__ . '/../../..' . '/lib/private/Preview/ProviderV2.php', 'OC\\Preview\\SGI' => __DIR__ . '/../../..' . '/lib/private/Preview/SGI.php', diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 94fea7314b565..375ddbe39301d 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -349,60 +349,74 @@ private function getMaxPreview(array $previews, File $file, string $mimeType, ?s * @throws NotFoundException */ private function generateProviderPreview(File $file, int $width, int $height, bool $crop, bool $max, string $mimeType, ?string $version): Preview { - $previewProviders = $this->previewManager->getProviders(); - foreach ($previewProviders as $supportedMimeType => $providers) { - // Filter out providers that does not support this mime + $entries = []; + foreach ($this->previewManager->getProviders() as $supportedMimeType => $providers) { if (!preg_match($supportedMimeType, $mimeType)) { continue; } foreach ($providers as $providerClosure) { - $provider = $this->helper->getProvider($providerClosure); if (!$provider) { continue; } - if (!$provider->isAvailable($file)) { continue; } + $entries[] = [ + 'class' => ltrim($provider::class, '\\'), + 'provider' => $provider, + ]; + } + } - $previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new'); - $sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency); - try { - $this->logger->debug('Calling preview provider for {mimeType} with width={width}, height={height}', [ - 'mimeType' => $mimeType, - 'width' => $width, - 'height' => $height, - ]); - $preview = $this->helper->getThumbnail($provider, $file, $width, $height); - } finally { - self::unguardWithSemaphore($sem); - } + foreach ($entries as $entry) { + $provider = $entry['provider']; + $class = $entry['class']; - if (!($preview instanceof IImage)) { - continue; - } + $previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new'); + $sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency); + try { + $this->logger->debug('Calling preview provider {provider} for {mimeType} with width={width}, height={height}', [ + 'provider' => $class, + 'mimeType' => $mimeType, + 'width' => $width, + 'height' => $height, + ]); + $preview = $this->helper->getThumbnail($provider, $file, $width, $height); + } catch (\Throwable $e) { + $this->logger->warning('Preview provider {provider} failed for {mimeType}', [ + 'provider' => $class, + 'mimeType' => $mimeType, + 'exception' => $e, + ]); + continue; + } finally { + self::unguardWithSemaphore($sem); + } - try { - $previewEntry = new Preview(); - $previewEntry->generateId(); - $previewEntry->setFileId($file->getId()); - $previewEntry->setStorageId($file->getMountPoint()->getNumericStorageId()); - $previewEntry->setSourceMimeType($file->getMimeType()); - $previewEntry->setWidth($preview->width()); - $previewEntry->setHeight($preview->height()); - $previewEntry->setVersion($version); - $previewEntry->setMax($max); - $previewEntry->setCropped($crop); - $previewEntry->setEncrypted(false); - $previewEntry->setMimetype($preview->dataMimeType()); - $previewEntry->setEtag($file->getEtag()); - $previewEntry->setMtime((new \DateTime())->getTimestamp()); - return $this->savePreview($previewEntry, $preview); - } catch (NotPermittedException) { - throw new NotFoundException(); - } + if (!($preview instanceof IImage)) { + continue; + } + + try { + $previewEntry = new Preview(); + $previewEntry->generateId(); + $previewEntry->setFileId($file->getId()); + $previewEntry->setStorageId($file->getMountPoint()->getNumericStorageId()); + $previewEntry->setSourceMimeType($file->getMimeType()); + $previewEntry->setWidth($preview->width()); + $previewEntry->setHeight($preview->height()); + $previewEntry->setVersion($version); + $previewEntry->setMax($max); + $previewEntry->setCropped($crop); + $previewEntry->setEncrypted(false); + $previewEntry->setMimetype($preview->dataMimeType()); + $previewEntry->setEtag($file->getEtag()); + $previewEntry->setMtime((new \DateTime())->getTimestamp()); + return $this->savePreview($previewEntry, $preview); + } catch (NotPermittedException) { + throw new NotFoundException(); } } diff --git a/lib/private/Preview/PreviewProviderDefaults.php b/lib/private/Preview/PreviewProviderDefaults.php new file mode 100644 index 0000000000000..4f72d9bf53e92 --- /dev/null +++ b/lib/private/Preview/PreviewProviderDefaults.php @@ -0,0 +1,58 @@ + + */ + public static function getBuiltinDefaultProviders(): array { + return [ + MarkDown::class, + TXT::class, + OpenDocument::class, + PNG::class, + JPEG::class, + GIF::class, + BMP::class, + XBitmap::class, + Krita::class, + WebP::class, + AVIF::class, + ]; + } + + /** + * Nextcloud-encouraged provider list. + * + * Matches core defaults, plus Imaginary first when it is configured (server + * tuning / AIO). Native HEIC is appended as a fallback because Imaginary + * does not handle every HEIC/HEIF file. + * + * @return list + */ + public static function getRecommendedEnabledProviders(bool $imaginaryConfigured, bool $heicFallback = false): array { + $providers = self::getBuiltinDefaultProviders(); + if (!$imaginaryConfigured) { + return $providers; + } + $providers = array_merge([Imaginary::class], $providers); + if ($heicFallback) { + $providers[] = HEIC::class; + } + return array_values(array_unique($providers)); + } +} diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php index c3f780de39ecb..dfc5f12b92c6b 100644 --- a/lib/private/PreviewManager.php +++ b/lib/private/PreviewManager.php @@ -41,6 +41,7 @@ use OC\Preview\PNG; use OC\Preview\Postscript; use OC\Preview\PreviewMigrationService; +use OC\Preview\PreviewProviderDefaults; use OC\Preview\SGI; use OC\Preview\StarOffice; use OC\Preview\Storage\StorageFactory; @@ -77,6 +78,14 @@ class PreviewManager implements IPreview { /** @var array> $providers */ protected array $providers = []; + /** + * Registrations in insertion order, rebuilt into {@see $providers} using + * ``enabledPreviewProviders`` as key order (PHP maps keep insertion order). + * + * @var list + */ + private array $providerRegistrations = []; + /** @var array mime type => support status */ protected array $mimeTypeSupportMap = []; @@ -111,15 +120,16 @@ public function __construct( * @param string $mimeTypeRegex Regex with the mime types that are supported by this provider * @param ProviderClosure $callable */ - private function registerProviderClosure(string $mimeTypeRegex, Closure $callable): void { + private function registerProviderClosure(string $mimeTypeRegex, Closure $callable, ?string $class = null): void { if (!$this->enablePreviews) { return; } - if (!isset($this->providers[$mimeTypeRegex])) { - $this->providers[$mimeTypeRegex] = []; - } - $this->providers[$mimeTypeRegex][] = $callable; + $this->providerRegistrations[] = [ + 'regex' => $mimeTypeRegex, + 'class' => $class !== null && $class !== '' ? ltrim($class, '\\') : null, + 'callable' => $callable, + ]; $this->providerListDirty = true; } @@ -132,21 +142,57 @@ public function getProviders(): array { $this->registerCoreProviders(); $this->registerBootstrapProviders(); if ($this->providerListDirty) { - $keys = array_map('strlen', array_keys($this->providers)); - array_multisort($keys, SORT_DESC, $this->providers); + $this->rebuildProvidersMap(); $this->providerListDirty = false; } return $this->providers; } + /** + * Rebuild {@see $providers} so foreach order follows ``enabledPreviewProviders``. + * + * PHP arrays preserve insertion order. Each provider has its own MIME regex + * in core, so visiting map keys in whitelist order is try-order for Generator. + */ + private function rebuildProvidersMap(): void { + $priority = []; + foreach ($this->getEnabledDefaultProvider() as $index => $class) { + if (!is_string($class) || $class === '') { + continue; + } + $priority[ltrim($class, '\\')] = $index; + } + + $registrations = []; + foreach ($this->providerRegistrations as $index => $registration) { + $registration['index'] = $index; + $registrations[] = $registration; + } + usort($registrations, function (array $left, array $right) use ($priority): int { + $leftClass = $left['class']; + $rightClass = $right['class']; + $leftPriority = is_string($leftClass) && isset($priority[$leftClass]) ? $priority[$leftClass] : PHP_INT_MAX; + $rightPriority = is_string($rightClass) && isset($priority[$rightClass]) ? $priority[$rightClass] : PHP_INT_MAX; + if ($leftPriority !== $rightPriority) { + return $leftPriority <=> $rightPriority; + } + return $left['index'] <=> $right['index']; + }); + + $this->providers = []; + foreach ($registrations as $registration) { + $this->providers[$registration['regex']][] = $registration['callable']; + } + } + /** * Does the manager have any providers */ #[\Override] public function hasProviders(): bool { $this->registerCoreProviders(); - return !empty($this->providers); + return $this->providerRegistrations !== []; } private function getGenerator(): Generator { @@ -279,11 +325,15 @@ protected function getEnabledDefaultProvider(): array { AVIF::class, ]; - $this->defaultProviders = $this->config->getSystemValue('enabledPreviewProviders', array_merge([ - MarkDown::class, - TXT::class, - OpenDocument::class, - ], $imageProviders)); + $configured = $this->config->getSystemValue('enabledPreviewProviders', null); + if (!is_array($configured)) { + $this->defaultProviders = PreviewProviderDefaults::getRecommendedEnabledProviders( + $this->config->getSystemValueString('preview_imaginary_url', '') !== '', + $this->imagickSupport->hasExtension() && $this->imagickSupport->supportsFormat('HEIC'), + ); + } else { + $this->defaultProviders = $configured; + } if (in_array(Image::class, $this->defaultProviders, true)) { $this->defaultProviders = array_merge($this->defaultProviders, $imageProviders); @@ -302,7 +352,7 @@ protected function registerCoreProvider(string $class, string $mimeType, array $ $this->registerProviderClosure($mimeType, function () use ($class, $options): IProviderV2 { /** @var IProviderV2 $class */ return new $class($options); - }); + }, $class); } } @@ -429,13 +479,17 @@ private function registerBootstrapProviders(): void { } $this->loadedBootstrapProviders[$key] = null; - $this->registerProviderClosure($provider->getMimeTypeRegex(), function () use ($provider): IProviderV2|false { - try { - return $this->container->get($provider->getService()); - } catch (NotFoundExceptionInterface) { - return false; - } - }); + $this->registerProviderClosure( + $provider->getMimeTypeRegex(), + function () use ($provider): IProviderV2|false { + try { + return $this->container->get($provider->getService()); + } catch (NotFoundExceptionInterface) { + return false; + } + }, + $provider->getService(), + ); } } diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php index 9024d2d0b6887..336fb9870fdd8 100644 --- a/tests/lib/Preview/GeneratorTest.php +++ b/tests/lib/Preview/GeneratorTest.php @@ -399,6 +399,117 @@ public function testNoProvider(): void { $this->generator->getPreview($file, 100, 100); } + public function testTriesNextProviderWhenEarlierFails(): void { + $file = $this->getFile(42, 'myMimeType'); + + $this->previewMapper->method('getAvailablePreviews') + ->with($this->equalTo([42])) + ->willReturn([42 => []]); + $this->config->method('getSystemValueString')->willReturnCallback(fn ($key, $default) => $default); + $this->config->method('getSystemValueInt')->willReturnCallback(fn ($key, $default) => $default); + + $failing = $this->createMock(IProviderV2::class); + $failing->method('isAvailable')->willReturn(true); + $ok = $this->createMock(IProviderV2::class); + $ok->method('isAvailable')->willReturn(true); + $this->previewManager->method('isMimeSupported')->willReturn(true); + $this->previewManager->method('getProviders')->willReturn([ + '/myMimeType/' => ['failing', 'ok'], + ]); + $this->helper->method('getProvider')->willReturnOnConsecutiveCalls($failing, $ok); + + $image = $this->createMock(IImage::class); + $image->method('width')->willReturn(2048); + $image->method('height')->willReturn(2048); + $image->method('valid')->willReturn(true); + $image->method('dataMimeType')->willReturn('image/png'); + $image->method('data')->willReturn('my data'); + $this->helper->method('getThumbnail')->willReturnCallback(function ($provider) use ($failing, $image) { + if ($provider === $failing) { + throw new \RuntimeException('imaginary down'); + } + return $image; + }); + $this->helper->method('getImage')->willReturn($this->getMockImage(2048, 2048, 'my resized data')); + $this->previewMapper->method('insert')->willReturnCallback(fn (Preview $preview): Preview => $preview); + $this->previewMapper->method('update')->willReturnCallback(fn (Preview $preview): Preview => $preview); + $this->storageFactory->method('writePreview')->willReturn(1000); + + $result = $this->generator->getPreview($file, 100, 100); + $this->assertSame('256-256.png', $result->getName()); + } + + public function testTriesProvidersInEnabledPreviewProvidersOrder(): void { + $file = $this->getFile(42, 'image/heic'); + + $this->previewMapper->method('getAvailablePreviews') + ->with($this->equalTo([42])) + ->willReturn([42 => []]); + $this->config->method('getSystemValueString')->willReturnCallback(fn ($key, $default) => $default); + $this->config->method('getSystemValueInt')->willReturnCallback(fn ($key, $default) => $default); + + $imaginary = new class implements IProviderV2 { + public function getMimeType(): string { + return '/image\/heic/'; + } + public function isAvailable(\OCP\Files\FileInfo $file): bool { + return true; + } + public function getThumbnail(\OCP\Files\File $file, int $maxX, int $maxY): ?IImage { + return null; + } + }; + $heic = new class implements IProviderV2 { + public function getMimeType(): string { + return '/image\/heic/'; + } + public function isAvailable(\OCP\Files\FileInfo $file): bool { + return true; + } + public function getThumbnail(\OCP\Files\File $file, int $maxX, int $maxY): ?IImage { + return null; + } + }; + + $this->previewManager->method('isMimeSupported')->willReturn(true); + // PreviewManager emits this map in enabledPreviewProviders key order + // (PHP insertion order), not regex-length order. + $this->previewManager->method('getProviders')->willReturn([ + '/image\/heic/' => ['heic'], + '/(image\/(bmp|png|jpeg|heic|heif))/' => ['imaginary'], + ]); + $this->helper->method('getProvider')->willReturnCallback(function ($id) use ($imaginary, $heic) { + return match ($id) { + 'imaginary' => $imaginary, + 'heic' => $heic, + default => false, + }; + }); + + $image = $this->createMock(IImage::class); + $image->method('width')->willReturn(2048); + $image->method('height')->willReturn(2048); + $image->method('valid')->willReturn(true); + $image->method('dataMimeType')->willReturn('image/jpeg'); + $image->method('data')->willReturn('heic data'); + $tried = []; + $this->helper->method('getThumbnail')->willReturnCallback(function ($provider) use ($heic, $imaginary, $image, &$tried) { + $tried[] = $provider; + if ($provider === $heic) { + return $image; + } + $this->fail('Imaginary must not run first when HEIC is earlier in enabledPreviewProviders'); + }); + $this->helper->method('getImage')->willReturn($this->getMockImage(2048, 2048, 'resized')); + $this->previewMapper->method('insert')->willReturnCallback(fn (Preview $preview): Preview => $preview); + $this->previewMapper->method('update')->willReturnCallback(fn (Preview $preview): Preview => $preview); + $this->storageFactory->method('writePreview')->willReturn(1000); + + $result = $this->generator->getPreview($file, 100, 100); + $this->assertSame([$heic], $tried); + $this->assertSame('256-256.png', $result->getName()); + } + private function getMockImage(int $width, int $height, string $data = '') { $image = $this->createMock(IImage::class); $image->method('height')->willReturn($width); diff --git a/tests/lib/Preview/PreviewManagerProviderOrderTest.php b/tests/lib/Preview/PreviewManagerProviderOrderTest.php new file mode 100644 index 0000000000000..2ba3df1001f96 --- /dev/null +++ b/tests/lib/Preview/PreviewManagerProviderOrderTest.php @@ -0,0 +1,74 @@ +createMock(IConfig::class); + $config->method('getSystemValueBool')->willReturnCallback(fn (string $key, bool $default) => match ($key) { + 'enable_previews' => true, + default => $default, + }); + $config->method('getSystemValue')->willReturnCallback(function (string $key, mixed $default) { + if ($key === 'enabledPreviewProviders') { + return [HEIC::class, Imaginary::class, JPEG::class]; + } + return $default; + }); + $config->method('getSystemValueString')->willReturn(''); + + $imagick = $this->createMock(IMagickSupport::class); + $imagick->method('hasExtension')->willReturn(true); + $imagick->method('supportsFormat')->willReturn(true); + + $finder = $this->createMock(IBinaryFinder::class); + $finder->method('findBinaryPath')->willReturn(false); + + $coordinator = $this->createMock(Coordinator::class); + $coordinator->method('getRegistrationContext')->willReturn(null); + + $manager = new PreviewManager( + $config, + $this->createMock(IRootFolder::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(GeneratorHelper::class), + null, + $coordinator, + $this->createMock(ContainerInterface::class), + $finder, + $imagick, + ); + + $keys = array_keys($manager->getProviders()); + $heicIndex = array_search('/image\/(x-)?hei(f|c)/', $keys, true); + $imaginaryIndex = array_search(Imaginary::supportedMimeTypes(), $keys, true); + $jpegIndex = array_search('/image\/jpeg/', $keys, true); + + $this->assertNotFalse($heicIndex); + $this->assertNotFalse($imaginaryIndex); + $this->assertNotFalse($jpegIndex); + $this->assertLessThan($imaginaryIndex, $heicIndex); + $this->assertLessThan($jpegIndex, $imaginaryIndex); + } +}