Feature #101446
openPrevent re processing images with "same" processing instructions
0%
Description
Foremost I'm not sure if TYPO3 should change that. Still I try to explain our scenario and a possible solution. I'm fine if TYPO3 decides to reject the issue.
Environment:
TYPO3 11.5.23
PHP 8.1
Our scenario:
We had a small code that called applyProcessingInstructions()
of TYPO3\CMS\Extbase\Service\ImageService
The code looked like this:
$cropParam = ($crop)?'c':'';
$processingInstructions = [
'width' => $imageWidth . $cropParam,
'crop' => $this->getCropArea($imageObj, $cropVariant),
];
if ($forceRatio > 0) {
$processingInstructions['height'] = ($crop) ? $imageWidth / $forceRatio . $cropParam : null;
}
$processedImage = $this->imageService->applyProcessingInstructions($imageObj, $processingInstructions);
We than needed to make changes to the code base and ended up with the following code:
$cropParam = ($crop) ? 'c' : '';
$processingInstructions = [
'width' => $imageWidth . $cropParam,
'height' => null,
'crop' => $this->getCropArea($image, $cropVariant),
];
if ($forceRatio > 0 && $crop) {
$processingInstructions['height'] = ($imageWidth / $forceRatio) . $cropParam;
}
return $this->imageService->applyProcessingInstructions(
$image,
$processingInstructions
);
We didn't expect anything bad from that change. Once deployed our system went down due to heavy system load, caused by many gm (=graphics magick) processed.
We reverted the change and thanks to Hannes with his experience we found the root cause. We modified the $processingInstructions
that is used to create a hash in order to find entries from database table sys_file_processedfile
. Our modifications lead to a different array (height before, crop and height now always was set). That lead to a different hash leading to not finding processed images, leading to re-processing all images.
My proposal:
Add some streamlining to the incoming processing instructions. E.g. alphabetical sorting, array filter to filter out empty values like `null`. Something in that regard should prevent such issues for systems.
It forces an update wizard in order to update all existing entries with the sanitized and updated hash version in order to prevent re processing all images after system update.
Updated by Benni Mack about 1 year ago
- Status changed from New to Accepted
I agree and I know the problem:
I've put some work in a "pre-patch": https://review.typo3.org/c/Packages/TYPO3.CMS/+/81153
There is a comment that we should actually do a ksort() at some point, which I think is necessary, but then we'd need to re-process all processed files.
Updated by Daniel Siepmann about 1 year ago
Does the order actually change the processed files? Otherwise an update wizard could be created that iterates over all existing processed db entries. Each entry still has the config, this can be deserialized, normalized, written back, together with an updated configurationsha1.
That way the existing data can be migrated.
But maybe I'm missing something here and take things too easy.