Merge pull request #52360 from nextcloud/artonge/fix/use_preview_api_for_blurhash_generation

1 year ago · 1c518a291d
6 changed files with 64 additions and 120 deletions
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@ -2230,24 +2230,9 @@
    </InvalidReturnStatement>
  </file>
  <file src="lib/private/Preview/Generator.php">
-    <InvalidArgument>
-      <code><![CDATA[$maxPreviewImage]]></code>
-    </InvalidArgument>
    <LessSpecificReturnType>
      <code><![CDATA[null|string]]></code>
    </LessSpecificReturnType>
-    <MismatchingDocblockParamType>
-      <code><![CDATA[ISimpleFile]]></code>
-    </MismatchingDocblockParamType>
-    <UndefinedInterfaceMethod>
-      <code><![CDATA[height]]></code>
-      <code><![CDATA[height]]></code>
-      <code><![CDATA[preciseResizeCopy]]></code>
-      <code><![CDATA[resizeCopy]]></code>
-      <code><![CDATA[valid]]></code>
-      <code><![CDATA[width]]></code>
-      <code><![CDATA[width]]></code>
-    </UndefinedInterfaceMethod>
  </file>
  <file src="lib/private/Preview/ProviderV1Adapter.php">
    <InvalidReturnStatement>
--- a/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php
+++ b/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php
@ -19,6 +19,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;

 /**
@ -27,11 +28,14 @@ use OCP\Lock\LockedException;
 * @template-implements IEventListener<AMetadataEvent>
 */
 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
@ -64,7 +68,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;
 		}
@ -73,35 +79,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
 	 *
--- a/lib/private/Preview/Generator.php
+++ b/lib/private/Preview/Generator.php
@ -12,6 +12,7 @@ use OCP\Files\IAppData;
 use OCP\Files\InvalidPathException;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
+use OCP\Files\SimpleFS\InMemoryFile;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
@ -43,17 +44,19 @@ class Generator {
 	 * The cache is searched first and if nothing usable was found then a preview is
 	 * generated by one of the providers
 	 *
-	 * @param File $file
-	 * @param int $width
-	 * @param int $height
-	 * @param bool $crop
-	 * @param string $mode
-	 * @param string|null $mimeType
 	 * @return ISimpleFile
 	 * @throws NotFoundException
 	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 */
-	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
+	public function getPreview(
+		File $file,
+		int $width = -1,
+		int $height = -1,
+		bool $crop = false,
+		string $mode = IPreview::MODE_FILL,
+		?string $mimeType = null,
+		bool $cacheResult = true,
+	): ISimpleFile {
 		$specification = [
 			'width' => $width,
 			'height' => $height,
@ -78,23 +81,19 @@ class Generator {
 			'mode' => $mode,
 			'mimeType' => $mimeType,
 		]);
-		
+

 		// since we only ask for one preview, and the generate method return the last one it created, it returns the one we want
-		return $this->generatePreviews($file, [$specification], $mimeType);
+		return $this->generatePreviews($file, [$specification], $mimeType, $cacheResult);
 	}

 	/**
 	 * Generates previews of a file
 	 *
-	 * @param File $file
-	 * @param non-empty-array $specifications
-	 * @param string $mimeType
-	 * @return ISimpleFile the last preview that was generated
 	 * @throws NotFoundException
 	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 */
-	public function generatePreviews(File $file, array $specifications, $mimeType = null) {
+	public function generatePreviews(File $file, array $specifications, ?string $mimeType = null, bool $cacheResult = true): ISimpleFile {
 		//Make sure that we can read the file
 		if (!$file->isReadable()) {
 			$this->logger->warning('Cannot read file: {path}, skipping preview generation.', ['path' => $file->getPath()]);
@ -167,7 +166,7 @@ class Generator {
 					}

 					$this->logger->warning('Cached preview not found for file {path}, generating a new preview.', ['path' => $file->getPath()]);
-					$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;
 				}
@ -351,11 +350,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();
@ -490,19 +488,20 @@ class Generator {
 	}

 	/**
-	 * @param ISimpleFolder $previewFolder
-	 * @param ISimpleFile $maxPreview
-	 * @param int $width
-	 * @param int $height
-	 * @param bool $crop
-	 * @param int $maxWidth
-	 * @param int $maxHeight
-	 * @param string $prefix
-	 * @return ISimpleFile
 	 * @throws NotFoundException
 	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 */
-	private function generatePreview(ISimpleFolder $previewFolder, IImage $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $prefix) {
+	private function generatePreview(
+		ISimpleFolder $previewFolder,
+		IImage $maxPreview,
+		int $width,
+		int $height,
+		bool $crop,
+		int $maxWidth,
+		int $maxHeight,
+		string $prefix,
+		bool $cacheResult,
+	): ISimpleFile {
 		$preview = $maxPreview;
 		if (!$preview->valid()) {
 			throw new \InvalidArgumentException('Failed to generate preview, failed to load image');
@ -539,12 +538,14 @@ class Generator {

 		$path = $this->generatePath($width, $height, $crop, false, $preview->dataMimeType(), $prefix);
 		try {
-			$file = $previewFolder->newFile($path);
-			$file->putContent($preview->data());
+			if ($cacheResult) {
+				return $previewFolder->newFile($path, $preview->data());
+			} else {
+				return new InMemoryFile($path, $preview->data());
+			}
 		} catch (NotPermittedException $e) {
 			throw new NotFoundException();
 		}
-
 		return $file;
 	}

--- a/lib/private/PreviewManager.php
+++ b/lib/private/PreviewManager.php
@ -145,29 +145,20 @@ class PreviewManager implements IPreview {
 		return $this->generator;
 	}

-	/**
-	 * Returns a preview of a file
-	 *
-	 * The cache is searched first and if nothing usable was found then a preview is
-	 * generated by one of the providers
-	 *
-	 * @param File $file
-	 * @param int $width
-	 * @param int $height
-	 * @param bool $crop
-	 * @param string $mode
-	 * @param string $mimeType
-	 * @return ISimpleFile
-	 * @throws NotFoundException
-	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
-	 * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
-	 */
-	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
+	public function getPreview(
+		File $file,
+		$width = -1,
+		$height = -1,
+		$crop = false,
+		$mode = IPreview::MODE_FILL,
+		$mimeType = null,
+		bool $cacheResult = true,
+	): ISimpleFile {
 		$this->throwIfPreviewsDisabled();
 		$previewConcurrency = $this->getGenerator()->getNumConcurrentPreviews('preview_concurrency_all');
 		$sem = Generator::guardWithSemaphore(Generator::SEMAPHORE_ID_ALL, $previewConcurrency);
 		try {
-			$preview = $this->getGenerator()->getPreview($file, $width, $height, $crop, $mode, $mimeType);
+			$preview = $this->getGenerator()->getPreview($file, $width, $height, $crop, $mode, $mimeType, $cacheResult);
 		} finally {
 			Generator::unguardWithSemaphore($sem);
 		}
--- a/lib/public/IPreview.php
+++ b/lib/public/IPreview.php
@ -71,12 +71,14 @@ interface IPreview {
 	 * @param bool $crop
 	 * @param string $mode
 	 * @param string $mimeType To force a given mimetype for the file (files_versions needs this)
+	 * @param bool $cacheResult Whether or not to cache the preview on the filesystem. Default to true. Can be useful to set to false to limit the amount of stored previews.
 	 * @return ISimpleFile
 	 * @throws NotFoundException
 	 * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
 	 * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
+	 * @since 32.0.0 - getPreview($cacheResult) added the $cacheResult argument to the signature
 	 */
-	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null);
+	public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null, bool $cacheResult = true);

 	/**
 	 * Returns true if the passed mime type is supported
--- a/tests/lib/Preview/GeneratorTest.php
+++ b/tests/lib/Preview/GeneratorTest.php
@ -22,25 +22,25 @@ use OCP\Preview\IProviderV2;
 use Psr\Log\LoggerInterface;

 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 */
 	private $generator;

-	private LoggerInterface|\PHPUnit\Framework\MockObject\MockObject $logger;
+	private LoggerInterface&\PHPUnit\Framework\MockObject\MockObject $logger;

 	protected function setUp(): void {
 		parent::setUp();
@ -196,18 +196,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'))
@ -218,10 +210,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, null));