From 62c2525c19bd53d4ba5c968473b16728a91c8522 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 13 May 2025 11:15:55 +0200 Subject: [PATCH] fix(blurhash): Use preview API to generate the previews This allows to benefit from all the checks done by the preview API. This also use the newly introduced `cacheResult` argument to limit disk usage. Signed-off-by: Louis Chemineau --- build/psalm-baseline.xml | 21 ---------- .../Listener/GenerateBlurhashMetadata.php | 42 +++++-------------- lib/private/Preview/Generator.php | 10 ++--- tests/lib/Preview/GeneratorTest.php | 30 ++++--------- vendor-bin/psalm/composer.lock | 6 +-- 5 files changed, 26 insertions(+), 83 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 571e9c2ae94..26ed6476c3f 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1701,12 +1701,6 @@ - - - - - - @@ -2562,24 +2556,9 @@ - - - - - - - - - - - - - - - diff --git a/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php b/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php index 4e4232768f8..d32b96fb6fa 100644 --- a/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php +++ b/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php @@ -36,6 +36,7 @@ use OCP\Files\NotPermittedException; use OCP\FilesMetadata\AMetadataEvent; use OCP\FilesMetadata\Event\MetadataBackgroundEvent; use OCP\FilesMetadata\Event\MetadataLiveEvent; +use OCP\IPreview; use OCP\Lock\LockedException; /** @@ -44,11 +45,14 @@ use OCP\Lock\LockedException; * @template-implements IEventListener */ class GenerateBlurhashMetadata implements IEventListener { - private const RESIZE_BOXSIZE = 30; - private const COMPONENTS_X = 4; private const COMPONENTS_Y = 3; + public function __construct( + private IPreview $preview, + ) { + } + /** * @throws NotPermittedException * @throws GenericFileException @@ -81,7 +85,9 @@ class GenerateBlurhashMetadata implements IEventListener { return; } - $image = $this->resizedImageFromFile($file); + $preview = $this->preview->getPreview($file, 64, 64, cacheResult: false); + $image = @imagecreatefromstring($preview->getContent()); + if (!$image) { return; } @@ -90,35 +96,6 @@ class GenerateBlurhashMetadata implements IEventListener { ->setEtag('blurhash', $currentEtag); } - /** - * @param File $file - * - * @return GdImage|false - * @throws GenericFileException - * @throws NotPermittedException - * @throws LockedException - */ - private function resizedImageFromFile(File $file): GdImage|false { - $image = @imagecreatefromstring($file->getContent()); - if ($image === false) { - return false; - } - - $currX = imagesx($image); - $currY = imagesy($image); - - if ($currX > $currY) { - $newX = self::RESIZE_BOXSIZE; - $newY = intval($currY * $newX / $currX); - } else { - $newY = self::RESIZE_BOXSIZE; - $newX = intval($currX * $newY / $currY); - } - - $newImage = @imagescale($image, $newX, $newY); - return ($newImage !== false) ? $newImage : $image; - } - /** * @param GdImage $image * @@ -133,6 +110,7 @@ class GenerateBlurhashMetadata implements IEventListener { $row = []; for ($x = 0; $x < $width; ++$x) { $index = imagecolorat($image, $x, $y); + /** @psalm-suppress InvalidArgument false positive, GdImage is ok since 8.0 */ $colors = imagecolorsforindex($image, $index); $row[] = [$colors['red'], $colors['green'], $colors['blue']]; } diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index b9e14f513c9..81b39ae1686 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -189,7 +189,7 @@ class Generator { $maxPreviewImage = $this->helper->getImage($maxPreview); } - $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion, $cacheResult); // New file, augment our array $previewFiles[] = $preview; } @@ -368,11 +368,10 @@ class Generator { $path = $this->generatePath($preview->width(), $preview->height(), $crop, $max, $preview->dataMimeType(), $prefix); try { - $file = $previewFolder->newFile($path); if ($preview instanceof IStreamImage) { - $file->putContent($preview->resource()); + return $previewFolder->newFile($path, $preview->resource()); } else { - $file->putContent($preview->data()); + return $previewFolder->newFile($path, $preview->data()); } } catch (NotPermittedException $e) { throw new NotFoundException(); @@ -558,14 +557,13 @@ class Generator { $path = $this->generatePath($width, $height, $crop, false, $preview->dataMimeType(), $prefix); try { if ($cacheResult) { - $file = $previewFolder->newFile($path, $preview->data()); + return $previewFolder->newFile($path, $preview->data()); } else { return new InMemoryFile($path, $preview->data()); } } catch (NotPermittedException $e) { throw new NotFoundException(); } - return $file; } diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php index c7ffab27592..09ca3a00c6b 100644 --- a/tests/lib/Preview/GeneratorTest.php +++ b/tests/lib/Preview/GeneratorTest.php @@ -37,19 +37,19 @@ use OCP\Preview\BeforePreviewFetchedEvent; use OCP\Preview\IProviderV2; class GeneratorTest extends \Test\TestCase { - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IConfig&\PHPUnit\Framework\MockObject\MockObject */ private $config; - /** @var IPreview|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IPreview&\PHPUnit\Framework\MockObject\MockObject */ private $previewManager; - /** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IAppData&\PHPUnit\Framework\MockObject\MockObject */ private $appData; - /** @var GeneratorHelper|\PHPUnit\Framework\MockObject\MockObject */ + /** @var GeneratorHelper&\PHPUnit\Framework\MockObject\MockObject */ private $helper; - /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IEventDispatcher&\PHPUnit\Framework\MockObject\MockObject */ private $eventDispatcher; /** @var Generator */ @@ -207,18 +207,10 @@ class GeneratorTest extends \Test\TestCase { $previewFolder->method('getDirectoryListing') ->willReturn([]); $previewFolder->method('newFile') - ->willReturnCallback(function ($filename) use ($maxPreview, $previewFile) { - if ($filename === '2048-2048-max.png') { - return $maxPreview; - } elseif ($filename === '256-256.png') { - return $previewFile; - } - $this->fail('Unexpected file'); - }); - - $maxPreview->expects($this->once()) - ->method('putContent') - ->with($this->equalTo('my data')); + ->willReturnMap([ + ['2048-2048-max.png', 'my data', $maxPreview], + ['256-256.png', 'my resized data', $previewFile], + ]); $previewFolder->method('getFile') ->with($this->equalTo('256-256.png')) @@ -229,10 +221,6 @@ class GeneratorTest extends \Test\TestCase { ->with($this->equalTo($maxPreview)) ->willReturn($image); - $previewFile->expects($this->once()) - ->method('putContent') - ->with('my resized data'); - $this->eventDispatcher->expects($this->once()) ->method('dispatchTyped') ->with(new BeforePreviewFetchedEvent($file, 100, 100, false, IPreview::MODE_FILL)); diff --git a/vendor-bin/psalm/composer.lock b/vendor-bin/psalm/composer.lock index 8a9c5709aa2..5a12c96f484 100644 --- a/vendor-bin/psalm/composer.lock +++ b/vendor-bin/psalm/composer.lock @@ -2247,11 +2247,11 @@ "packages-dev": [], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": {}, "prefer-stable": false, "prefer-lowest": false, - "platform": [], - "platform-dev": [], + "platform": {}, + "platform-dev": {}, "platform-overrides": { "php": "8.0" },