Merge pull request #31339 from nextcloud/fix/preview-code-cleaning

Fix typing in OC\Preview

7 files changed, 58 insertions, 37 deletions

diff --git a/lib/private/Preview/Bitmap.php b/lib/private/Preview/Bitmap.php
index 57451da5725..3a4108664dd 100644
--- a/lib/private/Preview/Bitmap.php
+++ b/lib/private/Preview/Bitmap.php
@@ -29,7 +29,7 @@ namespace OC\Preview;
 use Imagick;
 use OCP\Files\File;
 use OCP\IImage;
-use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 
 /**
  * Creates a PNG preview using ImageMagick via the PECL extension
@@ -43,16 +43,25 @@ abstract class Bitmap extends ProviderV2 {
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		$tmpPath = $this->getLocalFile($file);
+		if ($tmpPath === false) {
+			\OC::$server->get(LoggerInterface::class)->error(
+				'Failed to get thumbnail for: ' . $file->getPath(),
+				['app' => 'core']
+			);
+			return null;
+		}
 
 		// Creates \Imagick object from bitmap or vector file
 		try {
 			$bp = $this->getResizedPreview($tmpPath, $maxX, $maxY);
 		} catch (\Exception $e) {
-			\OC::$server->getLogger()->logException($e, [
-				'message' => 'File: ' . $file->getPath() . ' Imagick says:',
-				'level' => ILogger::ERROR,
-				'app' => 'core',
-			]);
+			\OC::$server->get(LoggerInterface::class)->error(
+				'File: ' . $file->getPath() . ' Imagick says:',
+				[
+					'exception' => $e,
+					'app' => 'core',
+				]
+			);
 			return null;
 		}
 
diff --git a/lib/private/Preview/Bundled.php b/lib/private/Preview/Bundled.php
index f026d3259f5..e0551bd392b 100644
--- a/lib/private/Preview/Bundled.php
+++ b/lib/private/Preview/Bundled.php
@@ -30,9 +30,11 @@ use OCP\IImage;
  * Extracts a preview from files that embed them in an ZIP archive
  */
 abstract class Bundled extends ProviderV2 {
-	protected function extractThumbnail(File $file, $path): ?IImage {
+	protected function extractThumbnail(File $file, string $path): ?IImage {
 		$sourceTmp = \OC::$server->getTempManager()->getTemporaryFile();
 		$targetTmp = \OC::$server->getTempManager()->getTemporaryFile();
+		$this->tmpFiles[] = $sourceTmp;
+		$this->tmpFiles[] = $targetTmp;
 
 		try {
 			$content = $file->fopen('r');
diff --git a/lib/private/Preview/HEIC.php b/lib/private/Preview/HEIC.php
index 6601de238a9..7ce6b93ba3b 100644
--- a/lib/private/Preview/HEIC.php
+++ b/lib/private/Preview/HEIC.php
@@ -32,7 +32,7 @@ namespace OC\Preview;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
 use OCP\IImage;
-use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 
 /**
  * Creates a JPG preview using ImageMagick via the PECL extension
@@ -63,17 +63,26 @@ class HEIC extends ProviderV2 {
 		}
 
 		$tmpPath = $this->getLocalFile($file);
+		if ($tmpPath === false) {
+			\OC::$server->get(LoggerInterface::class)->error(
+				'Failed to get thumbnail for: ' . $file->getPath(),
+				['app' => 'core']
+			);
+			return null;
+		}
 
 		// Creates \Imagick object from the heic file
 		try {
 			$bp = $this->getResizedPreview($tmpPath, $maxX, $maxY);
 			$bp->setFormat('jpg');
 		} catch (\Exception $e) {
-			\OC::$server->getLogger()->logException($e, [
-				'message' => 'File: ' . $file->getPath() . ' Imagick says:',
-				'level' => ILogger::ERROR,
-				'app' => 'core',
-			]);
+			\OC::$server->get(LoggerInterface::class)->error(
+				'File: ' . $file->getPath() . ' Imagick says:',
+				[
+					'exception' => $e,
+					'app' => 'core',
+				]
+			);
 			return null;
 		}
 
@@ -109,7 +118,7 @@ class HEIC extends ProviderV2 {
 		$bp->setImageFormat('jpg');
 
 		$bp = $this->resize($bp, $maxX, $maxY);
-		
+
 		return $bp;
 	}
 
diff --git a/lib/private/Preview/Krita.php b/lib/private/Preview/Krita.php
index ec7be4b82cf..eb25db9928c 100644
--- a/lib/private/Preview/Krita.php
+++ b/lib/private/Preview/Krita.php
@@ -39,11 +39,11 @@ class Krita extends Bundled {
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		$image = $this->extractThumbnail($file, 'mergedimage.png');
-		if ($image->valid()) {
+		if (($image !== null) && $image->valid()) {
 			return $image;
 		}
 		$image = $this->extractThumbnail($file, 'preview.png');
-		if ($image->valid()) {
+		if (($image !== null) && $image->valid()) {
 			return $image;
 		}
 		return null;
diff --git a/lib/private/Preview/Movie.php b/lib/private/Preview/Movie.php
index a6e424caa2a..781cbad1954 100644
--- a/lib/private/Preview/Movie.php
+++ b/lib/private/Preview/Movie.php
@@ -99,11 +99,14 @@ class Movie extends ProviderV2 {
 		foreach ($sizeAttempts as $size) {
 			$absPath = $this->getLocalFile($file, $size);
 
-			$result = $this->generateThumbNail($maxX, $maxY, $absPath, 5);
-			if ($result === null) {
-				$result = $this->generateThumbNail($maxX, $maxY, $absPath, 1);
+			$result = null;
+			if (is_string($absPath)) {
+				$result = $this->generateThumbNail($maxX, $maxY, $absPath, 5);
 				if ($result === null) {
-					$result = $this->generateThumbNail($maxX, $maxY, $absPath, 0);
+					$result = $this->generateThumbNail($maxX, $maxY, $absPath, 1);
+					if ($result === null) {
+						$result = $this->generateThumbNail($maxX, $maxY, $absPath, 0);
+					}
 				}
 			}
 
@@ -117,25 +120,18 @@ class Movie extends ProviderV2 {
 		return $result;
 	}
 
-	/**
-	 * @param int $maxX
-	 * @param int $maxY
-	 * @param string $absPath
-	 * @param int $second
-	 * @return null|\OCP\IImage
-	 */
-	private function generateThumbNail($maxX, $maxY, $absPath, $second): ?IImage {
+	private function generateThumbNail(int $maxX, int $maxY, string $absPath, int $second): ?IImage {
 		$tmpPath = \OC::$server->getTempManager()->getTemporaryFile();
 
 		$binaryType = substr(strrchr($this->binary, '/'), 1);
 
 		if ($binaryType === 'avconv') {
-			$cmd = $this->binary . ' -y -ss ' . escapeshellarg($second) .
+			$cmd = $this->binary . ' -y -ss ' . escapeshellarg((string)$second) .
 				' -i ' . escapeshellarg($absPath) .
 				' -an -f mjpeg -vframes 1 -vsync 1 ' . escapeshellarg($tmpPath) .
 				' 2>&1';
 		} elseif ($binaryType === 'ffmpeg') {
-			$cmd = $this->binary . ' -y -ss ' . escapeshellarg($second) .
+			$cmd = $this->binary . ' -y -ss ' . escapeshellarg((string)$second) .
 				' -i ' . escapeshellarg($absPath) .
 				' -f mjpeg -vframes 1' .
 				' ' . escapeshellarg($tmpPath) .
diff --git a/lib/private/Preview/OpenDocument.php b/lib/private/Preview/OpenDocument.php
index 8deb9fd0a1e..5f27e325d31 100644
--- a/lib/private/Preview/OpenDocument.php
+++ b/lib/private/Preview/OpenDocument.php
@@ -42,7 +42,7 @@ class OpenDocument extends Bundled {
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		$image = $this->extractThumbnail($file, 'Thumbnails/thumbnail.png');
-		if ($image->valid()) {
+		if (($image !== null) && $image->valid()) {
 			return $image;
 		}
 		return null;
diff --git a/lib/private/Preview/ProviderV2.php b/lib/private/Preview/ProviderV2.php
index 4323f149702..0cb7eb59e21 100644
--- a/lib/private/Preview/ProviderV2.php
+++ b/lib/private/Preview/ProviderV2.php
@@ -35,7 +35,7 @@ abstract class ProviderV2 implements IProviderV2 {
 	protected $options;
 
 	/** @var array */
-	private $tmpFiles = [];
+	protected $tmpFiles = [];
 
 	/**
 	 * Constructor
@@ -72,7 +72,7 @@ abstract class ProviderV2 implements IProviderV2 {
 	 */
 	abstract public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage;
 
-	protected function useTempFile(File $file) {
+	protected function useTempFile(File $file): bool {
 		return $file->isEncrypted() || !$file->getStorage()->isLocal();
 	}
 
@@ -81,9 +81,9 @@ abstract class ProviderV2 implements IProviderV2 {
 	 *
 	 * @param File $file
 	 * @param int $maxSize maximum size for temporary files
-	 * @return string
+	 * @return string|false
 	 */
-	protected function getLocalFile(File $file, int $maxSize = null): string {
+	protected function getLocalFile(File $file, int $maxSize = null) {
 		if ($this->useTempFile($file)) {
 			$absPath = \OC::$server->getTempManager()->getTemporaryFile();
 
@@ -97,14 +97,19 @@ abstract class ProviderV2 implements IProviderV2 {
 			$this->tmpFiles[] = $absPath;
 			return $absPath;
 		} else {
-			return $file->getStorage()->getLocalFile($file->getInternalPath());
+			$path = $file->getStorage()->getLocalFile($file->getInternalPath());
+			if (is_string($path)) {
+				return $path;
+			} else {
+				return false;
+			}
 		}
 	}
 
 	/**
 	 * Clean any generated temporary files
 	 */
-	protected function cleanTmpFiles() {
+	protected function cleanTmpFiles(): void {
 		foreach ($this->tmpFiles as $tmpFile) {
 			unlink($tmpFile);
 		}