Commit 81121750 authored by Helmut Hummel's avatar Helmut Hummel Committed by Georg Ringer
Browse files

[BUGFIX] Implement deferred BE image processing consistently

Change the implementation of backend deferred image processing
to use a file processor, which is only but always used in the backend.

By doing so all limitations of the current implementation are resolved,
which means, width and height of the image can be set in HTML, to avoid
layout shifts and once the images are processed, they will simply
be delivered by the web server.

Inconsistencies with thumbnail ratio (depending on crop being defined
or not) are also tackled on the go.

While this changes processing configuration for some files,
which triggers a re-generation, it should not matter, as image
processing will be done in parallel on request, making such changes
viable in a bugfix release.

The introduced database field is neither used nor required for the
current implementation, but is introduced to provide a possibility for
third party processors to replace the current implementation with simple
(and persisted) URLs to third party SaaS image processing services.

Resolves: #92188
Releases: master, 10.4
Change-Id: I8d1e14324085c5b6ba646475206c8cb10a1fc10d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65237


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Markus Klein's avatarMarkus Klein <markus.klein@typo3.org>
Tested-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
Reviewed-by: Markus Klein's avatarMarkus Klein <markus.klein@typo3.org>
Reviewed-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
parent 9738f3d0
<?php
declare(strict_types=1);
/*
* 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!
*/
namespace TYPO3\CMS\Backend\Controller\File;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use TYPO3\CMS\Core\Http\HtmlResponse;
use TYPO3\CMS\Core\Http\RedirectResponse;
use TYPO3\CMS\Core\Resource\Service\ImageProcessingService;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* @internal This class is a specific Backend controller implementation and is not considered part of the Public TYPO3 API.
*/
class ImageProcessController implements LoggerAwareInterface
{
use LoggerAwareTrait;
/**
* @var ImageProcessingService
*/
private $imageProcessingService;
public function __construct(ImageProcessingService $imageProcessingService)
{
$this->imageProcessingService = $imageProcessingService;
}
/**
* @param ServerRequestInterface $request
* @return ResponseInterface
*/
public function process(ServerRequestInterface $request): ResponseInterface
{
$processedFileId = (int)($request->getQueryParams()['id'] ?? 0);
try {
$processedFile = $this->imageProcessingService->process($processedFileId);
return new RedirectResponse(
GeneralUtility::locationHeaderUrl($processedFile->getPublicUrl(true))
);
} catch (\Throwable $e) {
// Fatal error occurred, which will be responded as 404
$this->logger->error(sprintf('Processing of file with id "%s" failed', $processedFileId), ['exception' => $e]);
}
return new HtmlResponse('', 404);
}
}
<?php
declare(strict_types=1);
/*
* 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!
*/
namespace TYPO3\CMS\Backend\Resource\Processing;
use TYPO3\CMS\Backend\Routing\UriBuilder;
use TYPO3\CMS\Core\Context\Context;
use TYPO3\CMS\Core\Imaging\ImageDimension;
use TYPO3\CMS\Core\Resource\ProcessedFileRepository;
use TYPO3\CMS\Core\Resource\Processing\ProcessorInterface;
use TYPO3\CMS\Core\Resource\Processing\TaskInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* @internal This class is a specific Backend controller implementation and is not considered part of the Public TYPO3 API.
*/
class DeferredBackendImageProcessor implements ProcessorInterface
{
public function canProcessTask(TaskInterface $task): bool
{
$context = GeneralUtility::makeInstance(Context::class);
return TYPO3_MODE === 'BE'
&& $task->getType() === 'Image'
&& in_array($task->getName(), ['Preview', 'CropScaleMask'], true)
&& (!$context->hasAspect('fileProcessing') || $context->getPropertyFromAspect('fileProcessing', 'deferProcessing'))
&& $task->getSourceFile()->getProperty('width') > 0
&& $task->getSourceFile()->getProperty('height') > 0
// Let the local image processor update the properties in case the target file exists already
&& !$task->getSourceFile()->getStorage()->getProcessingFolder()->hasFile($task->getTargetFileName());
}
public function processTask(TaskInterface $task): void
{
$imageDimension = ImageDimension::fromProcessingTask($task);
$processedFile = $task->getTargetFile();
if (!$processedFile->isPersisted()) {
// For now, we need to persist the processed file in the repository to be able to reference its uid
// We could instead introduce a processing queue and persist the information there
$processedFileRepository = GeneralUtility::makeInstance(ProcessedFileRepository::class);
$processedFileRepository->add($processedFile);
}
$processedFile->setName($task->getTargetFileName());
$processingUrl = (string)GeneralUtility::makeInstance(UriBuilder::class)
->buildUriFromRoute(
'image_processing',
[
'id' => $processedFile->getUid(),
]
);
$processedFile->updateProcessingUrl(GeneralUtility::locationHeaderUrl($processingUrl));
$processedFile->updateProperties(
[
'width' => $imageDimension->getWidth(),
'height' => $imageDimension->getHeight(),
'size' => 0,
'checksum' => $task->getConfigurationChecksum(),
]
);
$task->setExecuted(true);
}
}
......@@ -17,7 +17,6 @@ namespace TYPO3\CMS\Backend\Utility;
use Psr\Log\LoggerInterface;
use TYPO3\CMS\Backend\Configuration\TypoScript\ConditionMatching\ConditionMatcher;
use TYPO3\CMS\Backend\Controller\File\ThumbnailController;
use TYPO3\CMS\Backend\Routing\UriBuilder;
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
use TYPO3\CMS\Core\Cache\CacheManager;
......@@ -39,6 +38,7 @@ use TYPO3\CMS\Core\Domain\Repository\PageRepository;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
use TYPO3\CMS\Core\Imaging\Icon;
use TYPO3\CMS\Core\Imaging\IconFactory;
use TYPO3\CMS\Core\Imaging\ImageDimension;
use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Log\LogManager;
......@@ -1078,12 +1078,8 @@ class BackendUtility
$size = '',
$linkInfoPopup = true
) {
// Check and parse the size parameter
$size = trim((string)$size);
$sizeParts = [64, 64];
if ($size) {
$sizeParts = explode('x', $size . 'x' . $size);
}
$size = (int)(trim((string)$size) ?: 64);
$targetDimension = new ImageDimension($size, $size);
$thumbData = '';
$fileReferences = static::resolveFileReferences($table, $field, $row);
// FAL references
......@@ -1107,23 +1103,28 @@ class BackendUtility
// Preview web image or media elements
if ($GLOBALS['TYPO3_CONF_VARS']['GFX']['thumbnails']
&& GeneralUtility::inList(
$GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'],
$fileReferenceObject->getExtension()
)
&& $fileReferenceObject->getOriginalFile()->isImage()
) {
$cropVariantCollection = CropVariantCollection::create((string)$fileReferenceObject->getProperty('crop'));
$cropArea = $cropVariantCollection->getCropArea();
$imageUrl = self::getThumbnailUrl($fileObject->getUid(), [
'width' => $sizeParts[0],
'height' => $sizeParts[1] . 'c',
'crop' => $cropArea->isEmpty() ? null : $cropArea->makeAbsoluteBasedOnFile($fileReferenceObject),
'_context' => $cropArea->isEmpty() ? ProcessedFile::CONTEXT_IMAGEPREVIEW : ProcessedFile::CONTEXT_IMAGECROPSCALEMASK
]);
$taskType = ProcessedFile::CONTEXT_IMAGEPREVIEW;
$processingConfiguration = [
'width' => $targetDimension->getWidth(),
'height' => $targetDimension->getHeight(),
];
if (!$cropArea->isEmpty()) {
$taskType = ProcessedFile::CONTEXT_IMAGECROPSCALEMASK;
$processingConfiguration = [
'maxWidth' => $targetDimension->getWidth(),
'maxHeight' => $targetDimension->getHeight(),
'crop' => $cropArea->makeAbsoluteBasedOnFile($fileReferenceObject),
];
}
$processedImage = $fileObject->process($taskType, $processingConfiguration);
$attributes = [
'src' => $imageUrl,
'width' => (string)(int)$sizeParts[0],
'height' => (string)(int)$sizeParts[1],
'src' => $processedImage->getPublicUrl(true),
'width' => $processedImage->getProperty('width'),
'height' => $processedImage->getProperty('height'),
'alt' => $fileReferenceObject->getName(),
];
$imgTag = '<img ' . GeneralUtility::implodeAttributes($attributes, true) . $tparams . '/>';
......@@ -1152,23 +1153,16 @@ class BackendUtility
* @param int $fileId
* @param array $configuration
* @return string
* @throws \TYPO3\CMS\Backend\Routing\Exception\RouteNotFoundException
*/
public static function getThumbnailUrl(int $fileId, array $configuration): string
{
$parameters = (string)json_encode([
'fileId' => $fileId,
'configuration' => $configuration
]);
$uriParameters = [
'parameters' => $parameters,
'hmac' => GeneralUtility::hmac(
$parameters,
ThumbnailController::class
),
];
return (string)GeneralUtility::makeInstance(UriBuilder::class)
->buildUriFromRoute('thumbnails', $uriParameters);
$taskType = $configuration['_context'] ?? ProcessedFile::CONTEXT_IMAGEPREVIEW;
unset($configuration['_context']);
return GeneralUtility::makeInstance(ResourceFactory::class)
->getFileObject($fileId)
->process($taskType, $configuration)
->getPublicUrl(true);
}
/**
......
......@@ -15,7 +15,6 @@
namespace TYPO3\CMS\Backend\ViewHelpers;
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
use TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException;
use TYPO3\CMS\Core\Resource\ProcessedFile;
......@@ -95,9 +94,6 @@ class ThumbnailViewHelper extends ImageViewHelper
$cropVariant = $this->arguments['cropVariant'] ?: 'default';
$cropArea = $cropVariantCollection->getCropArea($cropVariant);
$processingInstructions = [];
if (!empty($this->arguments['context'])) {
$processingInstructions['_context'] = $this->arguments['context'];
}
if (!$cropArea->isEmpty()) {
$processingInstructions['crop'] = $cropArea->makeAbsoluteBasedOnFile($image);
}
......@@ -106,7 +102,9 @@ class ThumbnailViewHelper extends ImageViewHelper
$processingInstructions[$argument] = $this->arguments[$argument];
}
}
$imageUri = BackendUtility::getThumbnailUrl($image->getUid(), $processingInstructions);
$processedFile = $image->process($this->arguments['context'], $processingInstructions);
$imageUri = $processedFile->getPublicUrl(true);
if (!$this->tag->hasAttribute('data-focus-area')) {
$focusArea = $cropVariantCollection->getFocusArea($cropVariant);
......@@ -115,7 +113,8 @@ class ThumbnailViewHelper extends ImageViewHelper
}
}
$this->tag->addAttribute('src', $imageUri);
$this->tag->addAttribute('width', $this->arguments['maxWidth'] ?? $this->arguments['width']);
$this->tag->addAttribute('width', $processedFile->getProperty('width'));
$this->tag->addAttribute('height', $processedFile->getProperty('height'));
$alt = $image->getProperty('alternative');
$title = $image->getProperty('title');
......
......@@ -200,5 +200,11 @@ return [
'thumbnails' => [
'path' => '/thumbnails',
'target' => Controller\File\ThumbnailController::class . '::render'
]
],
// Image processing
'image_processing' => [
'path' => '/image/process',
'target' => Controller\File\ImageProcessController::class . '::process'
],
];
......@@ -27,6 +27,10 @@ services:
shared: false
public: true
TYPO3\CMS\Backend\Controller\File\ImageProcessController:
shared: false
public: true
TYPO3\CMS\Backend\View\PageLayoutView:
shared: false
public: true
......
<?php
declare(strict_types=1);
/*
* 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!
*/
namespace TYPO3\CMS\Core\Context;
use TYPO3\CMS\Core\Context\Exception\AspectPropertyNotFoundException;
/**
* The aspect contains the processed file representation that is requested to process locally.
*
* Allowed properties:
* - file
*/
class FileProcessingAspect implements AspectInterface
{
/**
* @var bool
*/
private $deferProcessing;
public function __construct(bool $deferProcessing = true)
{
$this->deferProcessing = $deferProcessing;
}
/**
* Fetch the values
*
* @param string $name
* @return bool
* @throws AspectPropertyNotFoundException
*/
public function get(string $name)
{
if ($name === 'deferProcessing') {
return $this->deferProcessing;
}
throw new AspectPropertyNotFoundException('Property "' . $name . '" not found in Aspect "' . __CLASS__ . '".', 1599164743);
}
public function isProcessingDeferred(): bool
{
return $this->deferProcessing;
}
}
<?php
/*
* 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!
*/
namespace TYPO3\CMS\Core\Locking;
use TYPO3\CMS\Core\Locking\Exception\LockAcquireException;
use TYPO3\CMS\Core\Locking\Exception\LockAcquireWouldBlockException;
use TYPO3\CMS\Core\Locking\Exception\LockCreateException;
/**
* Wrapper for locking API that uses two locks
* to not exhaust locking resources and still block properly
*/
class ResourceMutex
{
/**
* @var LockFactory
*/
private $lockFactory;
/**
* Access lock
*
* @var LockingStrategyInterface[]
*/
private $accessLocks = [];
/**
* Image processing lock
*
* @var LockingStrategyInterface[]
*/
private $workerLocks = [];
public function __construct(LockFactory $lockFactory)
{
$this->lockFactory = $lockFactory;
}
/**
* Acquire a specific lock for the given scope
* @see \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::acquireLock
*
* @param string $scope
* @param string $key
* @throws LockAcquireException
* @throws LockCreateException
*/
public function acquireLock(string $scope, string $key): void
{
$this->accessLocks[$scope] = $this->lockFactory->createLocker($scope);
$this->workerLocks[$scope] = $this->lockFactory->createLocker(
$key,
LockingStrategyInterface::LOCK_CAPABILITY_EXCLUSIVE | LockingStrategyInterface::LOCK_CAPABILITY_NOBLOCK
);
do {
if (!$this->accessLocks[$scope]->acquire()) {
throw new \RuntimeException('Could not acquire access lock for "' . $scope . '"".', 1601923209);
}
try {
$locked = $this->workerLocks[$scope]->acquire(
LockingStrategyInterface::LOCK_CAPABILITY_EXCLUSIVE | LockingStrategyInterface::LOCK_CAPABILITY_NOBLOCK
);
} catch (LockAcquireWouldBlockException $e) {
// somebody else has the lock, we keep waiting
// first release the access lock
$this->accessLocks[$scope]->release();
// now lets make a short break (100ms) until we try again, since
// the page generation by the lock owner will take a while anyways
usleep(100000);
continue;
}
$this->accessLocks[$scope]->release();
if ($locked) {
break;
}
throw new \RuntimeException('Could not acquire image process lock for ' . $key . '.', 1601923215);
} while (true);
}
/**
* Release a worker specific lock
*
* @param string $scope
* @throws LockAcquireException
* @throws LockAcquireWouldBlockException
*/
public function releaseLock(string $scope): void
{
if ($this->accessLocks[$scope] ?? null) {
if (!$this->accessLocks[$scope]->acquire()) {
throw new \RuntimeException('Could not acquire access lock for "' . $scope . '"".', 1601923319);
}
$this->workerLocks[$scope]->release();
$this->workerLocks[$scope]->destroy();
$this->workerLocks[$scope] = null;
$this->accessLocks[$scope]->release();
$this->accessLocks[$scope] = null;
}
}
}
<?php
declare(strict_types=1);
/*
* 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!
*/
namespace TYPO3\CMS\Core\Resource\Exception;
use TYPO3\CMS\Core\Resource\Exception;
use TYPO3\CMS\Core\Resource\ProcessedFile;
/**
* Exception indicating that a file is already processed
*
* @internal
*/
class FileAlreadyProcessedException extends Exception
{
/**
* @var ProcessedFile
*/
private $processedFile;
public function __construct(ProcessedFile $processedFile, int $code = 0)
{
$this->processedFile = $processedFile;
parent::__construct(sprintf('File "%s" has already been processed', $processedFile->getIdentifier()), $code);
}
/**
* @return ProcessedFile
*/
public function getProcessedFile(): ProcessedFile
{
return $this->processedFile;
}
}
......@@ -105,6 +105,14 @@ class ProcessedFile extends AbstractFile
*/
protected $updated = false;
/**
* If this is set, this URL is used as public URL
* This MUST be a fully qualified URL including host
*
* @var string
*/
protected $processingUrl = '';
/**
* Constructor for a processed file object. Should normally not be used
* directly, use the corresponding factory methods instead.
......@@ -141,6 +149,7 @@ class ProcessedFile extends AbstractFile
$this->identifier = $databaseRow['identifier'];
$this->name = $databaseRow['name'];
$this->properties = $databaseRow;
$this->processingUrl = $databaseRow['processing_url'] ?? '';
if (!empty($databaseRow['storage']) && (int)$this->storage->getUid() !== (int)$databaseRow['storage']) {
$this->storage = GeneralUtility::makeInstance(StorageRepository::class)->findByUid($databaseRow['storage']);
......@@ -350,6 +359,9 @@ class ProcessedFile extends AbstractFile
if (array_key_exists('uid', $properties) && MathUtility::canBeInterpretedAsInteger($properties['uid'])) {
$this->properties['uid'] = $properties['uid'];
}
if (isset($properties['processing_url'])) {
$this->processingUrl = $properties['processing_url'];
}
// @todo we should have a blacklist of properties that might not be updated
$this->properties = array_merge($this->properties, $properties);
......@@ -370,9 +382,9 @@ class ProcessedFile extends AbstractFile
if ($this->usesOriginalFile()) {
$properties = $this->originalFile->getProperties();
unset($properties['uid']);
unset($properties['pid']);
unset($properties['identifier']);
unset($properties['name']);
$properties['identifier'] = '';
$properties['name'] = null;
$properties['processing_url'] = '';
// Use width + height set in processed file
$properties['width'] = $this->properties['width'];
......@@ -413,9 +425,16 @@ class ProcessedFile extends AbstractFile
// @todo check if some of these properties can/should be set in a generic update method
$this->identifier = $this->originalFile->getIdentifier();
$this->updated = true;