Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not check for multiple image formats feature, add fallback #33407

Merged
merged 2 commits into from Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 9 additions & 13 deletions classes/Image.php
Expand Up @@ -24,9 +24,8 @@
* @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
*/

use PrestaShop\PrestaShop\Adapter\SymfonyContainer;
use PrestaShop\PrestaShop\Adapter\ServiceLocator;
use PrestaShop\PrestaShop\Core\Exception\InvalidArgumentException;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;

/**
Expand Down Expand Up @@ -638,16 +637,14 @@ public function deleteImage($forceDelete = false)

// Delete auto-generated images
$image_types = ImageType::getImagesTypes();
$sfContainer = SymfonyContainer::getInstance();
$isMultipleImageFormatFeatureEnabled = $sfContainer->get('prestashop.core.admin.feature_flag.repository')->isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT);

foreach ($image_types as $imageType) {
$filesToDelete = $this->deleteAutoGeneratedImage($imageType, $this->image_format, $filesToDelete);
// Get image formats we will be deleting. It would probably be easier to use ImageFormatConfiguration::SUPPORTED_FORMATS,
// but we want to avoid any behavior change in minor/patch version.
$configuredImageFormats = ServiceLocator::get(ImageFormatConfiguration::class)->getGenerationFormats();

if ($isMultipleImageFormatFeatureEnabled) {
foreach (ImageFormatConfiguration::SUPPORTED_FORMATS as $imageFormat) {
$filesToDelete = $this->deleteAutoGeneratedImage($imageType, $imageFormat, $filesToDelete);
}
foreach ($image_types as $imageType) {
foreach ($configuredImageFormats as $imageFormat) {
$filesToDelete = $this->deleteAutoGeneratedImage($imageType, $imageFormat, $filesToDelete);
}
}

Expand Down Expand Up @@ -948,11 +945,10 @@ public function getPathForCreation()
*/
private function deleteAutoGeneratedImage(array $imageType, string $imageFormat, array $filesToDelete): array
{
$configuration = SymfonyContainer::getInstance()->get('prestashop.adapter.legacy.configuration');
$filesToDelete[] = $this->image_dir . $this->getExistingImgPath() . '-' . $imageType['name'] . '.' . $imageFormat;
$filesToDelete[] = $this->image_dir . $this->getExistingImgPath() . '-' . $imageType['name'] . '2x.' . $imageFormat;
if ($configuration->get('WATERMARK_HASH')) {
$filesToDelete[] = $this->image_dir . $this->getExistingImgPath() . '-' . $imageType['name'] . '-' . $configuration->get('WATERMARK_HASH') . '.' . $imageFormat;
if (Configuration::get('WATERMARK_HASH')) {
$filesToDelete[] = $this->image_dir . $this->getExistingImgPath() . '-' . $imageType['name'] . '-' . Configuration::get('WATERMARK_HASH') . '.' . $imageFormat;
}

return $filesToDelete;
Expand Down
9 changes: 5 additions & 4 deletions classes/ObjectModel.php
Expand Up @@ -24,6 +24,7 @@
* @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
*/
use PrestaShop\PrestaShop\Adapter\ServiceLocator;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;
use PrestaShopBundle\Translation\TranslatorComponent;

abstract class ObjectModelCore implements \PrestaShop\PrestaShop\Core\Foundation\Database\EntityInterface
Expand Down Expand Up @@ -1965,11 +1966,11 @@ public function deleteImage($force_delete = false)
return false;
}

/** @var PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration $imageFormatConfiguration */
$imageFormatConfiguration = ServiceLocator::get('PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration');
$configuredImageFormats = $imageFormatConfiguration->getGenerationFormats();

// Get image formats we will be deleting. It would probably be easier to use ImageFormatConfiguration::SUPPORTED_FORMATS,
// but we want to avoid any behavior change in minor/patch version.
$configuredImageFormats = ServiceLocator::get(ImageFormatConfiguration::class)->getGenerationFormats();
$types = ImageType::getImagesTypes();

foreach ($types as $image_type) {
if (file_exists($this->image_dir . $this->id . '-' . stripslashes($image_type['name']) . '.' . $this->image_format)
&& !unlink($this->image_dir . $this->id . '-' . stripslashes($image_type['name']) . '.' . $this->image_format)) {
Expand Down
16 changes: 3 additions & 13 deletions controllers/admin/AdminImagesController.php
Expand Up @@ -98,7 +98,7 @@ public function init()

$this->canGenerateAvif = $this->get('PrestaShop\PrestaShop\Core\Image\AvifExtensionChecker')->isAvailable();
$this->isMultipleImageFormatFeatureEnabled = $this->get(FeatureFlagRepository::class)->isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT);
$this->imageFormatConfiguration = $this->get('PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration');
$this->imageFormatConfiguration = $this->get(ImageFormatConfiguration::class);

$formFields = [];

Expand Down Expand Up @@ -661,15 +661,10 @@ protected function _regenerateNewImages($dir, $type, $productsImages = false)

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
if ($this->isMultipleImageFormatFeatureEnabled) {
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();

if (!$productsImages) {
$formated_medium = ImageType::getFormattedName('medium');
Expand Down Expand Up @@ -817,15 +812,10 @@ protected function _regenerateNoPictureImages($dir, $type, $languages)

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
if ($this->isMultipleImageFormatFeatureEnabled) {
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();

foreach ($type as $image_type) {
foreach ($languages as $language) {
Expand Down
11 changes: 2 additions & 9 deletions controllers/admin/AdminProductsController.php
Expand Up @@ -24,7 +24,6 @@
* @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
*/
use PrestaShop\PrestaShop\Adapter\SymfonyContainer;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;

/**
Expand Down Expand Up @@ -2891,19 +2890,13 @@ public function ajaxProcessaddProductImage($idProduct = null, $inputFileName = '
// Should we generate high DPI images?
$generate_hight_dpi_images = (bool) Configuration::get('PS_HIGHT_DPI');

$sfContainer = SymfonyContainer::getInstance();

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
if ($sfContainer->get('prestashop.core.admin.feature_flag.repository')->isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT)) {
$configuredImageFormats = $sfContainer->get(ImageFormatConfiguration::class)->getGenerationFormats();
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = $this->get(ImageFormatConfiguration::class)->getGenerationFormats();

foreach ($imagesTypes as $imageType) {
foreach ($configuredImageFormats as $imageFormat) {
// For JPG images, we let Imagemanager decide what to do and choose between JPG/PNG.
Expand Down
10 changes: 1 addition & 9 deletions controllers/admin/AdminStoresController.php
Expand Up @@ -24,8 +24,6 @@
* @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
*/

use PrestaShop\PrestaShop\Adapter\SymfonyContainer;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;

/**
Expand Down Expand Up @@ -421,16 +419,10 @@ protected function postImage($id)

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
$sfContainer = SymfonyContainer::getInstance();
if ($sfContainer->get('prestashop.core.admin.feature_flag.repository')->isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT)) {
$configuredImageFormats = $sfContainer->get(ImageFormatConfiguration::class)->getGenerationFormats();
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = $this->get(ImageFormatConfiguration::class)->getGenerationFormats();

if (($id_store = (int) Tools::getValue('id_store')) && count($_FILES) && file_exists(_PS_STORE_IMG_DIR_ . $id_store . '.jpg')) {
$images_types = ImageType::getImagesTypes('stores');
Expand Down
12 changes: 5 additions & 7 deletions src/Adapter/Image/ImageGenerator.php
Expand Up @@ -31,7 +31,6 @@
use Configuration;
use ImageManager;
use ImageType;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;
use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException;
Expand All @@ -44,7 +43,11 @@
class ImageGenerator
{
/**
* @deprecated since 8.1.2, it was originally introduced in 8.1.0, but ended up no longer needed - will be removed in 9.0
*
* @var FeatureFlagRepository
*
* @phpstan-ignore-next-line
*/
private $featureFlagRepository;

Expand Down Expand Up @@ -105,15 +108,10 @@ protected function resize(string $filePath, ImageType $imageType, int $imageId =

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
if ($this->featureFlagRepository->isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT)) {
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = $this->imageFormatConfiguration->getGenerationFormats();

// Should we generate high DPI images?
$generate_high_dpi_images = (bool) Configuration::get('PS_HIGHT_DPI');
Expand Down
21 changes: 11 additions & 10 deletions src/Adapter/Image/ImageRetriever.php
Expand Up @@ -28,14 +28,14 @@

use Category;
use Configuration;
use FeatureFlag;
use Image;
use ImageManager;
use ImageType;
use Language;
use Link;
use Manufacturer;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Adapter\ServiceLocator;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;
use PrestaShopDatabaseException;
use PrestaShopException;
use Product;
Expand All @@ -52,12 +52,18 @@ class ImageRetriever
*/
private $link;

private $isMultipleImageFormatFeatureActive;
/**
* @deprecated since 8.1.2, it was originally introduced in 8.1.0, but ended up no longer needed - will be removed in 9.0
*
* @var bool
*
* @phpstan-ignore-next-line
*/
private $isMultipleImageFormatFeatureActive = false;

public function __construct(Link $link)
{
$this->link = $link;
$this->isMultipleImageFormatFeatureActive = FeatureFlag::isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT);
}

/**
Expand Down Expand Up @@ -185,15 +191,10 @@ public function getImage($object, $id_image)

/*
* Let's resolve which formats we will use for image generation.
* In new image system, it's multiple formats. In case of legacy, it's only .jpg.
*
* In case of .jpg images, the actual format inside is decided by ImageManager.
*/
if ($this->isMultipleImageFormatFeatureActive) {
$configuredImageFormats = explode(',', Configuration::get('PS_IMAGE_FORMAT'));
} else {
$configuredImageFormats = ['jpg'];
}
$configuredImageFormats = ServiceLocator::get(ImageFormatConfiguration::class)->getGenerationFormats();

// Primary (fake) image name is object rewrite, fallbacks are name and ID
if (!empty($object->link_rewrite)) {
Expand Down
7 changes: 3 additions & 4 deletions src/Adapter/Image/Uploader/ManufacturerImageUploader.php
Expand Up @@ -29,7 +29,9 @@
use Configuration;
use ImageManager;
use ImageType;
use PrestaShop\PrestaShop\Adapter\ServiceLocator;
use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;
use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException;
use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException;
use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface;
Expand Down Expand Up @@ -85,10 +87,7 @@ private function generateDifferentSizeImages($manufacturerId)
file_exists(_PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg')
) {
$imageTypes = ImageType::getImagesTypes('manufacturers');

/** @var \PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration $imageFormatConfiguration */
$imageFormatConfiguration = \PrestaShop\PrestaShop\Adapter\ServiceLocator::get('PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration');
$configuredImageFormats = $imageFormatConfiguration->getGenerationFormats();
$configuredImageFormats = ServiceLocator::get(ImageFormatConfiguration::class)->getGenerationFormats();

foreach ($imageTypes as $imageType) {
foreach ($configuredImageFormats as $imageFormat) {
Expand Down
Expand Up @@ -29,8 +29,10 @@
namespace PrestaShop\PrestaShop\Adapter\Manufacturer\CommandHandler;

use ImageType;
use PrestaShop\PrestaShop\Adapter\ServiceLocator;
use PrestaShop\PrestaShop\Core\Domain\Manufacturer\Command\DeleteManufacturerLogoImageCommand;
use PrestaShop\PrestaShop\Core\Domain\Manufacturer\CommandHandler\DeleteManufacturerLogoImageHandlerInterface;
use PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration;
use Symfony\Component\Filesystem\Filesystem;

/**
Expand Down Expand Up @@ -63,9 +65,9 @@ public function handle(DeleteManufacturerLogoImageCommand $command): void

$imageTypes = ImageType::getImagesTypes('manufacturers');

/** @var \PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration $imageFormatConfiguration */
$imageFormatConfiguration = \PrestaShop\PrestaShop\Adapter\ServiceLocator::get('PrestaShop\PrestaShop\Core\Image\ImageFormatConfiguration');
$configuredImageFormats = $imageFormatConfiguration->getGenerationFormats();
// Get image formats we will be deleting. It would probably be easier to use ImageFormatConfiguration::SUPPORTED_FORMATS,
// but we want to avoid any behavior change in minor/patch version.
$configuredImageFormats = ServiceLocator::get(ImageFormatConfiguration::class)->getGenerationFormats();

foreach ($imageTypes as $imageType) {
foreach ($configuredImageFormats as $imageFormat) {
Expand Down
1 change: 1 addition & 0 deletions src/Core/ContainerBuilder.php
Expand Up @@ -45,6 +45,7 @@ public function build()
$container->bind('PrestaShop\\PrestaShop\\Core\\ConfigurationInterface', '\\PrestaShop\\PrestaShop\\Adapter\\Configuration', true);
$container->bind('\\PrestaShop\\PrestaShop\\Core\\Foundation\\Database\\DatabaseInterface', '\\PrestaShop\\PrestaShop\\Adapter\\Database', true);
$container->bind('PrestaShop\\PrestaShop\\Core\\Foundation\\Database\\DatabaseInterface', '\\PrestaShop\\PrestaShop\\Adapter\\Database', true);
$container->bind('PrestaShop\\PrestaShop\\Core\\Image\\ImageFormatConfiguration', 'PrestaShop\\PrestaShop\\Core\\Image\\ImageFormatConfiguration', true);

return $container;
}
Expand Down
31 changes: 30 additions & 1 deletion src/Core/Image/ImageFormatConfiguration.php
Expand Up @@ -28,7 +28,9 @@

namespace PrestaShop\PrestaShop\Core\Image;

use FeatureFlag;
use PrestaShop\PrestaShop\Core\ConfigurationInterface;
use PrestaShop\PrestaShop\Core\FeatureFlag\FeatureFlagSettings;
use PrestaShop\PrestaShop\Core\Image\Exception\ImageFormatConfigurationException;

class ImageFormatConfiguration implements ImageFormatConfigurationInterface
Expand All @@ -39,6 +41,10 @@ class ImageFormatConfiguration implements ImageFormatConfigurationInterface

public const SUPPORTED_FORMATS = ['jpg', 'png', 'webp', 'avif'];

public const DEFAULT_IMAGE_FORMAT = 'jpg';

private $formatsToGenerate = [];

/**
* @var ConfigurationInterface
*/
Expand All @@ -51,7 +57,30 @@ public function __construct(ConfigurationInterface $configuration)

public function getGenerationFormats(): array
{
return explode(self::SEPARATOR, $this->configuration->get(self::IMAGE_FORMAT_CONFIGURATION_KEY));
// Return formats from cache
if (!empty($this->formatsToGenerate)) {
return $this->formatsToGenerate;
}

// We will start with the base format, that will be generated no matter what
$this->formatsToGenerate = [self::DEFAULT_IMAGE_FORMAT];

// If multiple image formats feature is not enabled, we return only the fallback format
if (!FeatureFlag::isEnabled(FeatureFlagSettings::FEATURE_FLAG_MULTIPLE_IMAGE_FORMAT)) {
return $this->formatsToGenerate;
}

// If it is enabled, we check for configured formats.
$configuration = $this->configuration->get(self::IMAGE_FORMAT_CONFIGURATION_KEY);
if (!empty($configuration)) {
foreach (explode(self::SEPARATOR, $configuration) as $format) {
if (in_array($format, self::SUPPORTED_FORMATS) && !in_array($format, $this->formatsToGenerate)) {
$this->formatsToGenerate[] = $format;
}
}
}

return $this->formatsToGenerate;
}

public function addGenerationFormat(string $format): void
Expand Down