[BUGFIX] Generate thumbnails for files without width/height 77/39077/5
authorMarkus Klösges <mkloesges@gmx.de>
Thu, 30 Apr 2015 10:06:32 +0000 (12:06 +0200)
committerMarkus Klein <markus.klein@typo3.org>
Thu, 21 May 2015 16:30:36 +0000 (18:30 +0200)
Thumbnails should not upscale images, therefore we check for the
original size against thumbnail size. If original size is 0 (as for
pdf-Files) this leads to not generating thumbnails for those files. Now
checking original width/height against 0 to prevent this and generate
thumbs for those files again.

Resolves: #66679
Related: #66270
Releases: master, 6.2
Change-Id: I5823142a6be19e6068fea358027dc808f5fd3185
Reviewed-on: http://review.typo3.org/39077
Tested-by: Markus Klösges <mkloesges@gmx.de>
Reviewed-by: Frans Saris <franssaris@gmail.com>
Tested-by: Frans Saris <franssaris@gmail.com>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
typo3/sysext/core/Classes/Resource/Processing/LocalPreviewHelper.php
typo3/sysext/core/Tests/Unit/Resource/Processing/LocalPreviewHelperTest.php [new file with mode: 0644]

index 46d6b97..e7e904d 100644 (file)
@@ -58,36 +58,57 @@ class LocalPreviewHelper {
        public function process(TaskInterface $task) {
                $sourceFile = $task->getSourceFile();
 
-                       // Merge custom configuration with default configuration
+               // Merge custom configuration with default configuration
                $configuration = array_merge(array('width' => 64, 'height' => 64), $task->getConfiguration());
                $configuration['width'] = MathUtility::forceIntegerInRange($configuration['width'], 1);
                $configuration['height'] = MathUtility::forceIntegerInRange($configuration['height'], 1);
 
-               // Only scale down when new dimensions are smaller then existing image
-               if ($configuration['width'] > $sourceFile->getProperty('width')
+               // Do not scale up if the source file has a size and the target size is larger
+               if ($sourceFile->getProperty('width') > 0 && $sourceFile->getProperty('height') > 0
+                       && $configuration['width'] > $sourceFile->getProperty('width')
                        && $configuration['height'] > $sourceFile->getProperty('height')) {
                        return NULL;
                }
 
-               $originalFileName = $sourceFile->getForLocalProcessing(FALSE);
+               return $this->generatePreviewFromFile($sourceFile, $configuration, $this->getTemporaryFilePath($task));
+       }
+
+       /**
+        * Returns the path to a temporary file for processing
+        *
+        * @param TaskInterface $task
+        * @return string
+        */
+       protected function getTemporaryFilePath(TaskInterface $task) {
+               return GeneralUtility::tempnam('preview_', '.' . $task->getTargetFileExtension());
+       }
 
-                       // Create a temporaryFile
-               $temporaryFileName = GeneralUtility::tempnam('preview_', '.' . $task->getTargetFileExtension());
-                       // Check file extension
-               if ($sourceFile->getType() != File::FILETYPE_IMAGE &&
-                       !GeneralUtility::inList($GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'], $sourceFile->getExtension())) {
-                               // Create a default image
+       /**
+        * Generates a preview for a file
+        *
+        * @param File $file The source file
+        * @param array $configuration Processing configuration
+        * @param string $targetFilePath Output file path
+        * @return array|NULL
+        */
+       protected function generatePreviewFromFile(File $file, array $configuration, $targetFilePath) {
+               $originalFileName = $file->getForLocalProcessing(FALSE);
+
+               // Check file extension
+               if ($file->getType() != File::FILETYPE_IMAGE &&
+                       !GeneralUtility::inList($GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'], $file->getExtension())) {
+                       // Create a default image
                        $graphicalFunctions = GeneralUtility::makeInstance(GraphicalFunctions::class);
                        $graphicalFunctions->getTemporaryImageWithText(
-                               $temporaryFileName,
+                               $targetFilePath,
                                'Not imagefile!',
                                'No ext!',
-                               $sourceFile->getName()
+                               $file->getName()
                        );
                        $result = array(
-                               'filePath' => $temporaryFileName,
+                               'filePath' => $targetFilePath,
                        );
-               } elseif ($sourceFile->getExtension() === 'svg') {
+               } elseif ($file->getExtension() === 'svg') {
                        /** @var $gifBuilder \TYPO3\CMS\Frontend\Imaging\GifBuilder */
                        $gifBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Frontend\Imaging\GifBuilder::class);
                        $gifBuilder->init();
@@ -103,28 +124,27 @@ class LocalPreviewHelper {
                                // Create the temporary file
                        if ($GLOBALS['TYPO3_CONF_VARS']['GFX']['im']) {
                                $parameters = '-sample ' . $configuration['width'] . 'x' . $configuration['height'] . ' '
-                                       . CommandUtility::escapeShellArgument($originalFileName) . '[0] ' . CommandUtility::escapeShellArgument($temporaryFileName);
+                                       . CommandUtility::escapeShellArgument($originalFileName) . '[0] ' . CommandUtility::escapeShellArgument($targetFilePath);
 
                                $cmd = GeneralUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
                                CommandUtility::exec($cmd);
 
-                               if (!file_exists($temporaryFileName)) {
+                               if (!file_exists($targetFilePath)) {
                                        // Create a error gif
                                        $graphicalFunctions = GeneralUtility::makeInstance(GraphicalFunctions::class);
                                        $graphicalFunctions->getTemporaryImageWithText(
-                                               $temporaryFileName,
+                                               $targetFilePath,
                                                'No thumb',
                                                'generated!',
-                                               $sourceFile->getName()
+                                               $file->getName()
                                        );
                                }
                        }
                        $result = array(
-                               'filePath' => $temporaryFileName,
+                               'filePath' => $targetFilePath,
                        );
                }
 
                return $result;
        }
-
 }
diff --git a/typo3/sysext/core/Tests/Unit/Resource/Processing/LocalPreviewHelperTest.php b/typo3/sysext/core/Tests/Unit/Resource/Processing/LocalPreviewHelperTest.php
new file mode 100644 (file)
index 0000000..7f3fe9d
--- /dev/null
@@ -0,0 +1,105 @@
+<?php
+namespace TYPO3\CMS\Core\Tests\Unit\Resource\Processing;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Resource\File;
+use TYPO3\CMS\Core\Resource\Processing\LocalPreviewHelper;
+use TYPO3\CMS\Core\Resource\Processing\TaskInterface;
+use TYPO3\CMS\Core\Tests\UnitTestCase;
+
+/**
+ * Testcase for \TYPO3\CMS\Core\Resource\Processing\LocalPreviewHelper
+ */
+class LocalPreviewHelperTest extends UnitTestCase {
+
+       /**
+        * @test
+        */
+       public function processProvidesDefaultSizeIfNotConfigured() {
+               $file = $this->getMockBuilder(File::class)
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               // Use size slightly larger than default size to ensure processing
+               $file->expects($this->any())->method('getProperty')->will($this->returnValueMap(array(
+                       array('width', 65),
+                       array('height', 65),
+               )));
+
+               $task = $this->getMock(TaskInterface::class);
+               $task->expects($this->once())->method('getSourceFile')->willReturn($file);
+               $task->expects($this->once())->method('getConfiguration')->willReturn(array());
+
+               $localPreviewHelper = $this->getMockBuilder(LocalPreviewHelper::class)
+                       ->disableOriginalConstructor()
+                       ->setMethods(array('getTemporaryFilePath', 'generatePreviewFromFile'))
+                       ->getMock();
+               $localPreviewHelper->expects($this->once())->method('getTemporaryFilePath')->willReturn('test/file');
+               // Assert that by default 64x64 is used as preview size
+               $localPreviewHelper->expects($this->once())->method('generatePreviewFromFile')
+                       ->with($file, array('width' => 64, 'height' => 64), 'test/file');
+
+               $localPreviewHelper->process($task);
+       }
+
+       /**
+        * @test
+        */
+       public function processDoesNotScaleUpImages() {
+               $file = $this->getMockBuilder(File::class)
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $file->expects($this->any())->method('getProperty')->will($this->returnValueMap(array(
+                       array('width', 20),
+                       array('height', 20),
+               )));
+
+               $localPreviewHelper = $this->getMockBuilder(LocalPreviewHelper::class)
+                       ->disableOriginalConstructor()
+                       ->setMethods(array('dummy'))
+                       ->getMock();
+
+               $task = $this->getMock(TaskInterface::class);
+               $task->expects($this->once())->method('getSourceFile')->willReturn($file);
+               $task->expects($this->once())->method('getConfiguration')->willReturn(array('width' => 30, 'height' => 30));
+
+               $this->assertNull($localPreviewHelper->process($task));
+       }
+
+       /**
+        * @test
+        */
+       public function processGeneratesPreviewEvenIfSourceFileHasNoSize() {
+               $file = $this->getMockBuilder(File::class)
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $file->expects($this->any())->method('getProperty')->will($this->returnValueMap(array(
+                       array('width', 0),
+                       array('height', 0),
+               )));
+
+               $task = $this->getMock(TaskInterface::class);
+               $task->expects($this->once())->method('getSourceFile')->willReturn($file);
+               $task->expects($this->once())->method('getConfiguration')->willReturn(array());
+
+               $localPreviewHelper = $this->getMockBuilder(LocalPreviewHelper::class)
+                       ->disableOriginalConstructor()
+                       ->setMethods(array('getTemporaryFilePath', 'generatePreviewFromFile'))
+                       ->getMock();
+               $expectedResult = array('width' => 20, 'height' => 20, 'filePath' => 'test/file');
+               $localPreviewHelper->expects($this->once())->method('generatePreviewFromFile')->willReturn($expectedResult);
+
+               $this->assertEquals($expectedResult, $localPreviewHelper->process($task));
+       }
+}