Skip to content

Commit a5b5d61

Browse files
wazumwazum
committed
[FEATURE] Add logging, rename configuration, cleanup and optimization
Ignore files in fallback storage (uid = 0). Warn in log if the converted file’s size is larger than the original. See README!
1 parent 0dd9d20 commit a5b5d61

10 files changed

+116
-71
lines changed

‎Classes/Adapter/AdapterInterface.php ‎Classes/Converter/Converter.php

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,24 @@
11
<?php
22
declare(strict_types=1);
33

4-
namespace Plan2net\Webp\Adapter;
4+
namespace Plan2net\Webp\Converter;
55

66
use RuntimeException;
77

88
/**
9-
* Interface AdapterInterface
9+
* Interface Converter
1010
*
11-
* @package Plan2net\Webp\Adapter
11+
* @package Plan2net\Webp\Converter
1212
*/
13-
interface AdapterInterface
13+
interface Converter
1414
{
1515
/**
16-
* AdapterInterface constructor.
1716
* @param string $parameters
1817
*/
1918
public function __construct(string $parameters);
2019

2120
/**
22-
* Convert a file $originalFilePath to webp in $targetFilePath.
21+
* Converts a file $originalFilePath to webp in $targetFilePath.
2322
*
2423
* @param string $originalFilePath
2524
* @param string $targetFilePath

‎Classes/Adapter/ExternalAdapter.php ‎Classes/Converter/ExternalConverter.php

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22
declare(strict_types=1);
33

4-
namespace Plan2net\Webp\Adapter;
4+
namespace Plan2net\Webp\Converter;
55

66
use InvalidArgumentException;
77
use RuntimeException;
@@ -10,11 +10,12 @@
1010

1111
/**
1212
* Class ExternalAdapter
13+
* Uses an external binary (e.g. cwebp)
1314
*
1415
* @package Plan2net\Webp\Adapter
1516
* @author Wolfgang Klinger <[email protected]>
1617
*/
17-
class ExternalAdapter implements AdapterInterface
18+
class ExternalConverter implements Converter
1819
{
1920
/**
2021
* @var string
@@ -28,7 +29,7 @@ class ExternalAdapter implements AdapterInterface
2829
public function __construct(string $parameters)
2930
{
3031
if (substr_count($parameters, '%s') !== 2) {
31-
throw new InvalidArgumentException('Command string is invalid, supply 2 string placeholders!');
32+
throw new InvalidArgumentException('Command string is invalid, supply 2 string (%s) placeholders!');
3233
}
3334
$binary = explode(' ', $parameters)[0];
3435
if (!is_executable($binary)) {
@@ -48,7 +49,7 @@ public function convert(string $originalFilePath, string $targetFilePath)
4849
GeneralUtility::fixPermissions($targetFilePath);
4950

5051
if (!@is_file($targetFilePath)) {
51-
throw new RuntimeException(sprintf('File "%s" could not be created!', $targetFilePath));
52+
throw new RuntimeException(sprintf('File "%s" was not created!', $targetFilePath));
5253
}
5354
}
5455
}

‎Classes/Adapter/MagickAdapter.php ‎Classes/Converter/MagickConverter.php

+9-7
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
<?php
22
declare(strict_types=1);
33

4-
namespace Plan2net\Webp\Adapter;
4+
namespace Plan2net\Webp\Converter;
55

66
use TYPO3\CMS\Core\Imaging\GraphicalFunctions;
77
use TYPO3\CMS\Core\Utility\GeneralUtility;
88

99
/**
1010
* Class MagickAdapter
11+
* Uses the builtin TYPO3 graphical functions (imagemagick, graphicsmagick)
1112
*
12-
* @package Plan2net\Webp\Adapter
13+
* @package Plan2net\Webp\Converter
1314
* @author Wolfgang Klinger <[email protected]>
1415
*/
15-
class MagickAdapter implements AdapterInterface
16+
class MagickConverter implements Converter
1617
{
1718
/**
1819
* @var
@@ -32,14 +33,17 @@ public function __construct(string $parameters)
3233
*/
3334
public function convert(string $originalFilePath, string $targetFilePath)
3435
{
35-
$this->getGraphicalFunctionsObject()->imageMagickExec(
36+
$result = $this->getGraphicalFunctionsObject()->imageMagickExec(
3637
$originalFilePath,
3738
$targetFilePath,
3839
$this->parameters
3940
);
4041

4142
if (!@is_file($targetFilePath)) {
42-
throw new \RuntimeException(sprintf('File %s could not be created!', $targetFilePath));
43+
throw new \RuntimeException(sprintf('File "%s" was not created: %s!',
44+
$targetFilePath,
45+
$result ?: 'maybe missing support for webp?'
46+
));
4347
}
4448
}
4549

@@ -53,8 +57,6 @@ protected function getGraphicalFunctionsObject(): GraphicalFunctions
5357
if ($graphicalFunctionsObject === null) {
5458
/** @var GraphicalFunctions $graphicalFunctionsObject */
5559
$graphicalFunctionsObject = GeneralUtility::makeInstance(GraphicalFunctions::class);
56-
// @todo remove (TYPO3 CMS 9.5)
57-
$graphicalFunctionsObject->init();
5860
}
5961

6062
return $graphicalFunctionsObject;

‎Classes/Processing/Webp.php

+29-15
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
use Plan2net\Webp\Service\Configuration;
77
use Plan2net\Webp\Service\Webp as WebpService;
8+
use TYPO3\CMS\Core\Log\LogManager;
89
use TYPO3\CMS\Core\Resource\Driver\DriverInterface;
910
use TYPO3\CMS\Core\Resource\File;
1011
use TYPO3\CMS\Core\Resource\ProcessedFile;
@@ -21,14 +22,14 @@
2122
class Webp
2223
{
2324
/**
24-
* Process a file using the specified adapter to create a webp copy.
25+
* Process a file using the configured adapter to create a webp copy
2526
*
2627
* @param FileProcessingService $fileProcessingService
27-
* @param DriverInterface $driver
28-
* @param ProcessedFile $processedFile
29-
* @param File $file
30-
* @param string $taskType
31-
* @param array $configuration
28+
* @param DriverInterface $driver
29+
* @param ProcessedFile $processedFile
30+
* @param File $file
31+
* @param string $taskType
32+
* @param array $configuration
3233
*/
3334
public function processFile(
3435
FileProcessingService $fileProcessingService,
@@ -60,7 +61,7 @@ public function processFile(
6061
$service = GeneralUtility::makeInstance(WebpService::class);
6162
$service->process($processedFile, $processedFileWebp);
6263

63-
// @todo using shortMD5 results in a very bad checksum,
64+
// Be aware that using shortMD5 results in a very bad checksum,
6465
// but TYPO3 CMS core has a limit on this field
6566
$processedFileWebp->updateProperties(
6667
[
@@ -72,7 +73,16 @@ public function processFile(
7273
// This will add or update
7374
$processedFileRepository->add($processedFileWebp);
7475
} catch (\Exception $e) {
75-
// @todo log
76+
$logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
77+
$logger->error(sprintf('Failed to convert image to webp: %s', $e->getMessage()));
78+
try {
79+
$processedFile->delete(true);
80+
} catch (\Exception $e) {
81+
$logger->error(sprintf('Failed to remove processed file "%s": %s',
82+
$processedFile->getIdentifier(),
83+
$e->getMessage()
84+
));
85+
}
7686
}
7787
}
7888
}
@@ -84,26 +94,26 @@ public function processFile(
8494
*/
8595
protected function shouldProcess(string $taskType, ProcessedFile $processedFile): bool
8696
{
87-
$process = true;
88-
8997
if ($taskType !== 'Image.CropScaleMask') {
90-
$process = false;
98+
return false;
9199
}
92100

93101
if (!WebpService::isSupportedMimeType($processedFile->getMimeType())) {
94-
$process = false;
102+
return false;
95103
}
104+
96105
// Convert images in any folder or only in the _processed_ folder
97106
$convertAllImages = (bool)Configuration::get('convert_all');
98107
if (!$convertAllImages && !$this->isFileInProcessingFolder($processedFile)) {
99-
$process = false;
108+
return false;
100109
}
110+
101111
// Process files only in a local and writable storage
102112
if (!$this->isStorageLocalAndWritable($processedFile)) {
103-
$process = false;
113+
return false;
104114
}
105115

106-
return $process;
116+
return true;
107117
}
108118

109119
/**
@@ -135,6 +145,10 @@ protected function isFileInProcessingFolder($file): bool
135145
protected function isStorageLocalAndWritable($file): bool
136146
{
137147
$storage = $file->getStorage();
148+
// Ignore files in fallback storage (e.g. files from extensions)
149+
if ($storage->getStorageRecord()['uid'] === 0) {
150+
return false;
151+
}
138152

139153
return $storage->getDriverType() === 'Local' && $storage->isWritable();
140154
}

‎Classes/Service/Configuration.php

+11-5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
namespace Plan2net\Webp\Service;
55

66
use TYPO3\CMS\Core\SingletonInterface;
7+
use TYPO3\CMS\Core\Utility\GeneralUtility;
78

89
/**
910
* Class Configuration
@@ -19,16 +20,21 @@ class Configuration implements SingletonInterface
1920
protected static $configuration = [];
2021

2122
/**
22-
* Returns the whole extension configuration or a specific property
23+
* Returns the whole extension configuration or a specific key
2324
*
2425
* @param string|null $key
2526
* @return array|string|null
2627
*/
27-
public static function get($key = null)
28+
public static function get(?string $key = null)
2829
{
29-
if (empty(self::$configuration) && isset($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['webp'])) {
30-
self::$configuration = (array)unserialize($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['webp'], [false]);
31-
}
30+
try {
31+
if (version_compare(TYPO3_version, '9.5', '>=')) {
32+
self::$configuration = GeneralUtility::makeInstance(\TYPO3\CMS\Core\Configuration\ExtensionConfiguration::class)->get('webp');
33+
} else {
34+
// @deprecated, remove when dropping support for 8.7
35+
self::$configuration = (array)unserialize($GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['webp'], [false]);
36+
}
37+
} catch (\Exception $e) {}
3238

3339
if (!empty($key)) {
3440
if (isset(self::$configuration[$key])) {

‎Classes/Service/Webp.php

+27-24
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
namespace Plan2net\Webp\Service;
55

6-
use Exception;
7-
use Plan2net\Webp\Adapter\AdapterInterface;
6+
use InvalidArgumentException;
7+
use Plan2net\Webp\Converter\Converter;
8+
use RuntimeException;
9+
use TYPO3\CMS\Core\Log\LogManager;
810
use TYPO3\CMS\Core\Resource\ProcessedFile;
911
use TYPO3\CMS\Core\Utility\GeneralUtility;
1012

@@ -26,8 +28,10 @@ class Webp
2628
*
2729
* @param ProcessedFile $originalFile
2830
* @param ProcessedFile $processedFile
31+
* @throws InvalidArgumentException
32+
* @throws RuntimeException
2933
*/
30-
public function process($originalFile, $processedFile)
34+
public function process(ProcessedFile $originalFile, ProcessedFile $processedFile)
3135
{
3236
$processedFile->setName($originalFile->getName() . '.webp');
3337
$processedFile->setIdentifier($originalFile->getIdentifier() . '.webp');
@@ -38,38 +42,37 @@ public function process($originalFile, $processedFile)
3842
// and don't need another copy
3943
$targetFilePath = $processedFile->getForLocalProcessing(false);
4044

41-
$adapterClass = Configuration::get('adapter');
45+
$converterClass = Configuration::get('converter');
4246
$parameters = $this->getParametersForMimeType($originalFile->getMimeType());
43-
if ($parameters) {
44-
try {
45-
/** @var AdapterInterface $adapter */
46-
$adapter = GeneralUtility::makeInstance($adapterClass, $parameters);
47-
$adapter->convert(
48-
$originalFilePath,
49-
$targetFilePath
47+
if (!empty($parameters)) {
48+
/** @var Converter $converter */
49+
$converter = GeneralUtility::makeInstance($converterClass, $parameters);
50+
$converter->convert($originalFilePath, $targetFilePath);
51+
$fileSizeTargetFile = @filesize($targetFilePath);
52+
if ($originalFile->getSize() <= $fileSizeTargetFile) {
53+
$logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
54+
$logger->warning(
55+
sprintf('Processed file (%s) is larger than the original (%s)!',
56+
$targetFilePath, $originalFilePath)
5057
);
51-
$processedFile->updateProperties(
52-
[
53-
'width' => $originalFile->getProperty('width'),
54-
'height' => $originalFile->getProperty('height'),
55-
'size' => @filesize($targetFilePath)
56-
]
57-
);
58-
} catch (Exception $e) {
59-
$processedFile->delete();
60-
// @todo log
6158
}
59+
$processedFile->updateProperties(
60+
[
61+
'width' => $originalFile->getProperty('width'),
62+
'height' => $originalFile->getProperty('height'),
63+
'size' => $fileSizeTargetFile
64+
]
65+
);
6266
} else {
63-
$processedFile->delete();
64-
// @todo log
67+
throw new InvalidArgumentException(sprintf('No options given for adapter "%s"!', $converterClass));
6568
}
6669
}
6770

6871
/**
6972
* @param $mimeType
7073
* @return bool
7174
*/
72-
public static function isSupportedMimeType($mimeType): bool
75+
public static function isSupportedMimeType(string $mimeType): bool
7376
{
7477
return in_array(strtolower($mimeType), self::SUPPORTED_MIME_TYPES, true);
7578
}

0 commit comments

Comments
 (0)