[BUGFIX] Generate thumbnails for files without width/height 55/39655/2
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:48:27 +0000 (18:48 +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/39655
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 63567c1..79f8cf7 100644 (file)
@@ -14,7 +14,10 @@ namespace TYPO3\CMS\Core\Resource\Processing;
  * The TYPO3 project - inspiring people to share!
  */
 
-use \TYPO3\CMS\Core\Resource, \TYPO3\CMS\Core\Utility;
+use TYPO3\CMS\Core\Utility\MathUtility;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Resource\File;
+use TYPO3\CMS\Core\Utility\CommandUtility;
 
 /**
  * Helper for creating local image previews using TYPO3s image processing classes.
@@ -44,47 +47,69 @@ class LocalPreviewHelper {
         * @return array|NULL
         */
        public function process(TaskInterface $task) {
-               $targetFile = $task->getTargetFile();
                $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'] = Utility\MathUtility::forceIntegerInRange($configuration['width'], 1);
-               $configuration['height'] = Utility\MathUtility::forceIntegerInRange($configuration['height'], 1);
+               $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')
-                       && $configuration['height'] > $sourceFile->getProperty('height')) {
+               // 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());
+       }
+
+       /**
+        * 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);
 
-                       // Create a temporaryFile
-               $temporaryFileName = Utility\GeneralUtility::tempnam('preview_', '.' . $task->getTargetFileExtension());
-                       // Check file extension
-               if ($sourceFile->getType() != Resource\File::FILETYPE_IMAGE &&
-                       !Utility\GeneralUtility::inList($GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'], $sourceFile->getExtension())) {
-                               // Create a default image
-                       $this->processor->getTemporaryImageWithText($temporaryFileName, 'Not imagefile!', 'No ext!', $sourceFile->getName());
+               // Check file extension
+               if ($file->getType() != File::FILETYPE_IMAGE &&
+                       !GeneralUtility::inList($GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'], $file->getExtension())) {
+                       // Create a default image
+                       $this->processor->getTemporaryImageWithText($targetFilePath, 'Not imagefile!', 'No ext!', $file->getName());
                } else {
                                // Create the temporary file
                        if ($GLOBALS['TYPO3_CONF_VARS']['GFX']['im']) {
                                $parameters = '-sample ' . $configuration['width'] . 'x' . $configuration['height'] . ' '
-                                       . $this->processor->wrapFileName($originalFileName) . '[0] ' . $this->processor->wrapFileName($temporaryFileName);
+                                       . $this->processor->wrapFileName($originalFileName) . '[0] ' . $this->processor->wrapFileName($targetFilePath);
 
-                               $cmd = Utility\GeneralUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
-                               Utility\CommandUtility::exec($cmd);
+                               $cmd = GeneralUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
+                               CommandUtility::exec($cmd);
 
-                               if (!file_exists($temporaryFileName)) {
-                                               // Create a error gif
-                                       $this->processor->getTemporaryImageWithText($temporaryFileName, 'No thumb', 'generated!', $sourceFile->getName());
+                               if (!file_exists($targetFilePath)) {
+                                       // Create a error gif
+                                       $this->processor->getTemporaryImageWithText($targetFilePath, 'No thumb', 'generated!', $file->getName());
                                }
                        }
                }
 
                return array(
-                       'filePath' => $temporaryFileName,
+                       'filePath' => $targetFilePath,
                );
        }
 }
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..06901b9
--- /dev/null
@@ -0,0 +1,102 @@
+<?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\Tests\UnitTestCase;
+
+/**
+ * Testcase for \TYPO3\CMS\Core\Resource\Processing\LocalPreviewHelper
+ */
+class LocalPreviewHelperTest extends UnitTestCase {
+
+       /**
+        * @test
+        */
+       public function processProvidesDefaultSizeIfNotConfigured() {
+               $file = $this->getMockBuilder('TYPO3\\CMS\\Core\\Resource\\File')
+                       ->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('TYPO3\\CMS\\Core\\Resource\\Processing\\TaskInterface');
+               $task->expects($this->once())->method('getSourceFile')->willReturn($file);
+               $task->expects($this->once())->method('getConfiguration')->willReturn(array());
+
+               $localPreviewHelper = $this->getMockBuilder('TYPO3\\CMS\\Core\\Resource\\Processing\\LocalPreviewHelper')
+                       ->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('TYPO3\\CMS\\Core\\Resource\\File')
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $file->expects($this->any())->method('getProperty')->will($this->returnValueMap(array(
+                       array('width', 20),
+                       array('height', 20),
+               )));
+
+               $localPreviewHelper = $this->getMockBuilder('TYPO3\\CMS\\Core\\Resource\\Processing\\LocalPreviewHelper')
+                       ->disableOriginalConstructor()
+                       ->setMethods(array('dummy'))
+                       ->getMock();
+
+               $task = $this->getMock('TYPO3\\CMS\\Core\\Resource\\Processing\\TaskInterface');
+               $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('TYPO3\\CMS\\Core\\Resource\\File')
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $file->expects($this->any())->method('getProperty')->will($this->returnValueMap(array(
+                       array('width', 0),
+                       array('height', 0),
+               )));
+
+               $task = $this->getMock('TYPO3\\CMS\\Core\\Resource\\Processing\\TaskInterface');
+               $task->expects($this->once())->method('getSourceFile')->willReturn($file);
+               $task->expects($this->once())->method('getConfiguration')->willReturn(array());
+
+               $localPreviewHelper = $this->getMockBuilder('TYPO3\\CMS\\Core\\Resource\\Processing\\LocalPreviewHelper')
+                       ->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));
+       }
+}