From e6a1fa70e236ec3193b0fd54dfcf4800902f519d Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 13:55:12 +0300 Subject: [PATCH 01/87] introduce product ImageId & constraint exception --- .../ProductImageConstraintException.php | 40 ++++++++++ .../Image/Exception/ProductImageException.php | 38 ++++++++++ .../Product/Image/ValueObject/ImageId.php | 74 +++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php create mode 100644 src/Core/Domain/Product/Image/Exception/ProductImageException.php create mode 100644 src/Core/Domain/Product/Image/ValueObject/ImageId.php diff --git a/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php b/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php new file mode 100644 index 0000000000000..090e9af706eb3 --- /dev/null +++ b/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php @@ -0,0 +1,40 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception; + +/** + * Is thrown when product image constraints are violated + */ +class ProductImageConstraintException extends ProductImageException +{ + /** + * When image id is invalid + */ + const INVALID_ID = 10; +} diff --git a/src/Core/Domain/Product/Image/Exception/ProductImageException.php b/src/Core/Domain/Product/Image/Exception/ProductImageException.php new file mode 100644 index 0000000000000..a2a49a052278a --- /dev/null +++ b/src/Core/Domain/Product/Image/Exception/ProductImageException.php @@ -0,0 +1,38 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception; + +use PrestaShop\PrestaShop\Core\Domain\Product\Exception\ProductException; + +/** + * Base exception for product image subdomain + */ +class ProductImageException extends ProductException +{ +} diff --git a/src/Core/Domain/Product/Image/ValueObject/ImageId.php b/src/Core/Domain/Product/Image/ValueObject/ImageId.php new file mode 100644 index 0000000000000..d6a3c18731705 --- /dev/null +++ b/src/Core/Domain/Product/Image/ValueObject/ImageId.php @@ -0,0 +1,74 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject; + +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageConstraintException; + +/** + * Holds image identification + */ +class ImageId +{ + /** + * @var int + */ + private $value; + + /** + * @param int $value + */ + public function __construct(int $value) + { + $this->assertIntegerIsGreaterThanZero($value); + $this->value = $value; + } + + /** + * @return int + */ + public function getValue(): int + { + return $this->value; + } + + /** + * @param int $value + * + * @throws ProductImageConstraintException + */ + private function assertIntegerIsGreaterThanZero(int $value): void + { + if ($value <= 0) { + throw new ProductImageConstraintException( + sprintf('Invalid image id "%d"', $value), + ProductImageConstraintException::INVALID_ID + ); + } + } +} From 68ab4b6fc423313133814b4cbcd41867d16b650e Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 14:13:44 +0300 Subject: [PATCH 02/87] prepare handler for AddProductImage --- .../CommandHandler/AddProductImageHandler.php | 47 +++++++ .../Command/AddProductImageCommand.php | 123 ++++++++++++++++++ .../AddProductImageHandlerInterface.php | 43 ++++++ .../config/services/adapter/product.yml | 7 + 4 files changed, 220 insertions(+) create mode 100644 src/Adapter/Product/CommandHandler/AddProductImageHandler.php create mode 100644 src/Core/Domain/Product/Command/AddProductImageCommand.php create mode 100644 src/Core/Domain/Product/CommandHandler/AddProductImageHandlerInterface.php diff --git a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php new file mode 100644 index 0000000000000..f9ed5b7d89bed --- /dev/null +++ b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php @@ -0,0 +1,47 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; + +use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; +use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\AddProductImageHandlerInterface; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; + +/** + * Handles @see AddProductImageCommand + */ +final class AddProductImageHandler implements AddProductImageHandlerInterface +{ + /** + * {@inheritdoc} + */ + public function handle(AddProductImageCommand $command): ImageId + { + // TODO: Implement handle() method. + } +} diff --git a/src/Core/Domain/Product/Command/AddProductImageCommand.php b/src/Core/Domain/Product/Command/AddProductImageCommand.php new file mode 100644 index 0000000000000..b304486f745ce --- /dev/null +++ b/src/Core/Domain/Product/Command/AddProductImageCommand.php @@ -0,0 +1,123 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Command; + +use PrestaShop\PrestaShop\Core\Domain\Product\ValueObject\ProductId; + +/** + * Adds new product image + */ +class AddProductImageCommand +{ + /** + * @var ProductId + */ + private $productId; + + /** + * @var string + */ + private $originalName; + + /** + * @var string + */ + private $pathName; + + /** + * @var int + */ + private $fileSize; + + /** + * @var string + */ + private $mimeType; + + /** + * @param ProductId $productId + * @param string $originalName + * @param string $pathName + * @param int $fileSize + * @param string $mimeType + */ + public function __construct( + ProductId $productId, + string $originalName, + string $pathName, + int $fileSize, + string $mimeType + ) { + $this->productId = $productId; + $this->originalName = $originalName; + $this->pathName = $pathName; + $this->fileSize = $fileSize; + $this->mimeType = $mimeType; + } + + /** + * @return ProductId + */ + public function getProductId(): ProductId + { + return $this->productId; + } + + /** + * @return string + */ + public function getOriginalName(): string + { + return $this->originalName; + } + + /** + * @return string + */ + public function getPathName(): string + { + return $this->pathName; + } + + /** + * @return int + */ + public function getFileSize(): int + { + return $this->fileSize; + } + + /** + * @return string + */ + public function getMimeType(): string + { + return $this->mimeType; + } +} diff --git a/src/Core/Domain/Product/CommandHandler/AddProductImageHandlerInterface.php b/src/Core/Domain/Product/CommandHandler/AddProductImageHandlerInterface.php new file mode 100644 index 0000000000000..12f37f32c54c3 --- /dev/null +++ b/src/Core/Domain/Product/CommandHandler/AddProductImageHandlerInterface.php @@ -0,0 +1,43 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +namespace PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler; + +use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; + +/** + * Defines contract to handle @see AddProductImageCommand + */ +interface AddProductImageHandlerInterface +{ + /** + * @param AddProductImageCommand $command + * + * @return ImageId + */ + public function handle(AddProductImageCommand $command): ImageId; +} diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index bbc22835596b4..dfbb8f1c1c6d4 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -391,6 +391,7 @@ services: arguments: - '@prestashop.adapter.product.repository.product_repository' +<<<<<<< HEAD prestashop.adapter.product.command_handler.duplicate_product_handler: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\DuplicateProductHandler arguments: @@ -408,3 +409,9 @@ services: - '@prestashop.adapter.shop.context' - '@translator' - '@prestashop.core.util.string.string_modifier' + + prestashop.adapter.product.command_handler.add_product_image_handler: + class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\AddProductImageHandler + tags: + - name: tactician.handler + command: PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand From fa1fb7c5b1af9994c455b7a15db2002b60f29df3 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 15:53:23 +0300 Subject: [PATCH 03/87] add ProductImageUploader (WIP) --- .../Image/Uploader/ProductImageUploader.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 src/Adapter/Image/Uploader/ProductImageUploader.php diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php new file mode 100644 index 0000000000000..5a49d33e35952 --- /dev/null +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -0,0 +1,34 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; + +class ProductImageUploader +{ + +} From bd828ddf92619cab6a38d651791509bc9e6ca663 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 15:53:59 +0300 Subject: [PATCH 04/87] add ProductImagePathFactory --- .../Product/Image/ProductImagePathFactory.php | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/Adapter/Product/Image/ProductImagePathFactory.php diff --git a/src/Adapter/Product/Image/ProductImagePathFactory.php b/src/Adapter/Product/Image/ProductImagePathFactory.php new file mode 100644 index 0000000000000..c6a943b5d761e --- /dev/null +++ b/src/Adapter/Product/Image/ProductImagePathFactory.php @@ -0,0 +1,77 @@ + + * @copyright 2007-2020 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + * International Registered Trademark & Property of PrestaShop SA + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Product\Image; + +use Image; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; + +class ProductImagePathFactory +{ + /** + * @var bool + */ + private $isLegacyImageMode; + + /** + * @param bool $isLegacyImageMode + */ + public function __construct( + bool $isLegacyImageMode + ) { + $this->isLegacyImageMode = $isLegacyImageMode; + } + + public function getBasePath(Image $image, bool $withExtension): string + { + if ($this->isLegacyImageMode) { + $path = $image->id_product . '-' . $image->id; + } else { + $path = $image->getImgPath(); + } + + //@todo: it seems that jpg is hardcoded. AdminProductsController:2836 + if ($withExtension) { + $path .= sprintf('.%s', $image->image_format); + } + + return _PS_PROD_IMG_DIR_ . $path; + } + + public function createDestinationDirectory(Image $image): void + { + if ($this->isLegacyImageMode || $image->createImgFolder()) { + return; + } + + throw new ImageUploadException(sprintf( + 'Error occurred when trying to create directory for product #%s image', + $image->id_product + )); + } +} From f7239caffc99951feaf95ed2bd559895e1300be1 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 15:54:25 +0300 Subject: [PATCH 05/87] remove implementing interface in abstract class. (BC BREAK) --- .../Image/Uploader/AbstractImageUploader.php | 3 +- .../Uploader/CategoryCoverImageUploader.php | 3 +- .../CategoryThumbnailImageUploader.php | 3 +- .../Image/Uploader/EmployeeImageUploader.php | 3 +- .../Uploader/ManufacturerImageUploader.php | 3 +- .../Image/Uploader/ProductImageUploader.php | 165 +++++++++++++++++- .../Image/Uploader/SupplierImageUploader.php | 3 +- 7 files changed, 169 insertions(+), 14 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index f730d9c89c57e..aac0315481039 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -33,7 +33,6 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; -use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use PrestaShopException; use Symfony\Component\HttpFoundation\File\UploadedFile; use Tools; @@ -43,7 +42,7 @@ * * @internal */ -abstract class AbstractImageUploader implements ImageUploaderInterface +abstract class AbstractImageUploader { /** * Check if image is allowed to be uploaded. diff --git a/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php b/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php index c9792814761b1..205db6ad5a49e 100644 --- a/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php +++ b/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php @@ -33,6 +33,7 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; /** @@ -40,7 +41,7 @@ * * @internal */ -final class CategoryCoverImageUploader extends AbstractImageUploader +final class CategoryCoverImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * {@inheritdoc} diff --git a/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php b/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php index 01b99ca59b5b8..a45c0aef14e0f 100644 --- a/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php +++ b/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php @@ -31,12 +31,13 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Class CategoryThumbnailImageUploader. */ -final class CategoryThumbnailImageUploader extends AbstractImageUploader +final class CategoryThumbnailImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * {@inheritdoc} diff --git a/src/Adapter/Image/Uploader/EmployeeImageUploader.php b/src/Adapter/Image/Uploader/EmployeeImageUploader.php index fb1cc427a3221..68eb8ea71698b 100644 --- a/src/Adapter/Image/Uploader/EmployeeImageUploader.php +++ b/src/Adapter/Image/Uploader/EmployeeImageUploader.php @@ -29,12 +29,13 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; use Employee; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Uploads employee logo image */ -final class EmployeeImageUploader extends AbstractImageUploader +final class EmployeeImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * @var string diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index acd1861c8328b..84ad2680114f4 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -32,13 +32,14 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use PrestaShopException; use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Uploads manufacturer logo image */ -final class ManufacturerImageUploader extends AbstractImageUploader +final class ManufacturerImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * {@inheritdoc} diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 5a49d33e35952..80803ece18d8a 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -1,12 +1,11 @@ - * @copyright Since 2007 PrestaShop SA and Contributors + * @author PrestaShop SA + * @copyright 2007-2020 PrestaShop SA and Contributors * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + * International Registered Trademark & Property of PrestaShop SA */ declare(strict_types=1); namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; -class ProductImageUploader +use ErrorException; +use Hook; +use Image; +use ImageManager; +use PrestaShop\PrestaShop\Adapter\Product\Image\ProductImagePathFactory; +use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\CannotUnlinkImageException; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ImageNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; + +final class ProductImageUploader extends AbstractImageUploader { + /** + * @var UploadSizeConfigurationInterface + */ + private $uploadSizeConfiguration; + + /** + * @var ProductImagePathFactory + */ + private $productImagePathFactory; + + /** + * @var array + */ + private $contextShopIdsList; + + /** + * @var int + */ + private $contextShopId; + + /** + * @param UploadSizeConfigurationInterface $uploadSizeConfiguration + * @param ProductImagePathFactory $productImagePathFactory + * @param array $contextShopIdsList + * @param int $contextShopId + */ + public function __construct( + UploadSizeConfigurationInterface $uploadSizeConfiguration, + ProductImagePathFactory $productImagePathFactory, + array $contextShopIdsList, + int $contextShopId + ) { + $this->uploadSizeConfiguration = $uploadSizeConfiguration; + $this->productImagePathFactory = $productImagePathFactory; + $this->contextShopIdsList = $contextShopIdsList; + $this->contextShopId = $contextShopId; + } + + /** + * {@inheritdoc} + */ + public function upload( + Image $image, + string $filePath, + string $format + ): void { + $this->checkMemory($filePath); + $this->productImagePathFactory->createDestinationDirectory($image); + $this->copyToDestination($filePath, $image); + + $this->generateDifferentSizeImages( + $this->productImagePathFactory->getBasePath($image, false), + 'products', + $format + ); + + Hook::exec('actionWatermark', ['id_image' => $image->id, 'id_product' => $image->id_product]); + //@Todo: wait for multishop specs + $image->associateTo($this->contextShopIdsList); + $this->deleteOldGeneratedImages($image); + } + + /** + * @param Image $image + * + * @throws CannotUnlinkImageException + */ + private function deleteOldGeneratedImages(Image $image): void + { + $oldGeneratedImages = [ + _PS_TMP_IMG_DIR_ . 'product_' . (int) $image->id . '.jpg', + _PS_TMP_IMG_DIR_ . 'product_mini_' . (int) $image->id_product . '_' . $this->contextShopId . '.jpg', + ]; + + foreach ($oldGeneratedImages as $oldImage) { + if (file_exists($oldImage)) { + try { + unlink($oldImage); + } catch (ErrorException $e) { + throw new CannotUnlinkImageException( + sprintf( + 'Failed to remove old generated image "%s"', + $oldImage + ), + 0, + $e + ); + } + } + } + } + + /** + * @param string $tmpImageName + * @param Image $image + * + * @throws ImageOptimizationException + */ + private function copyToDestination(string $tmpImageName, Image $image) + { + if (!ImageManager::resize($tmpImageName, $this->productImagePathFactory->getBasePath($image, true))) { + throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); + } + } + + /** + * Evaluate the memory required to resize the image: if it's too much, you can't resize it. + * + * @param string $tmpImageName + * + * @throws MemoryLimitException + */ + private function checkMemory(string $tmpImageName): void + { + if (!ImageManager::checkImageMemoryLimit($tmpImageName)) { + throw new MemoryLimitException('Due to memory limit restrictions, this image cannot be loaded. Increase your memory_limit value.'); + } + } + + /** + * @param ImageId $imageId + * + * @return Image + * + * @throws ImageNotFoundException + */ + private function loadImageEntity(ImageId $imageId): Image + { + $imageIdValue = $imageId->getValue(); + $image = new Image($imageIdValue); + + if ((int) $image->id !== $imageIdValue) { + throw new ImageNotFoundException(sprintf( + 'Image entity with id #%s does not exist', + $imageIdValue + )); + } + return $image; + } } diff --git a/src/Adapter/Image/Uploader/SupplierImageUploader.php b/src/Adapter/Image/Uploader/SupplierImageUploader.php index 7907b6fe9087b..4fdfc536622ba 100644 --- a/src/Adapter/Image/Uploader/SupplierImageUploader.php +++ b/src/Adapter/Image/Uploader/SupplierImageUploader.php @@ -26,13 +26,14 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Supplier; use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Uploads supplier logo image */ -final class SupplierImageUploader extends AbstractImageUploader +final class SupplierImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * {@inheritdoc} From b35eab8b27a2bc5d82b3f9447ea6f2d4d5d33f03 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 16:06:38 +0300 Subject: [PATCH 06/87] deprecate method in abstract uploader --- .../Image/Uploader/AbstractImageUploader.php | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index aac0315481039..a68ee2cba9098 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -1,12 +1,11 @@ - * @copyright Since 2007 PrestaShop SA and Contributors + * @author PrestaShop SA + * @copyright 2007-2020 PrestaShop SA and Contributors * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + * International Registered Trademark & Property of PrestaShop SA */ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; @@ -110,17 +110,27 @@ protected function uploadFromTemp($temporaryImageName, $destination) } /** - * Generates different size images - * - * @param int $id - * @param string $imageDir + * @deprecated use AbstractImageUploader::generateDifferentSizeImages() instead + */ + protected function generateDifferentSize($id, $imageDir, $belongsTo, string $extension = 'jpg') + { + return $this->generateDifferentSizeImages( + $imageDir . $id, + $belongsTo, + $extension + ); + } + + /** + * @param string $path * @param string $belongsTo to whom the image belongs (for example 'suppliers' or 'categories') + * @param string $extension * * @return bool * * @throws ImageOptimizationException */ - protected function generateDifferentSize($id, $imageDir, $belongsTo) + protected function generateDifferentSizeImages(string $path, string $belongsTo, string $extension = 'jpg'): bool { $resized = true; @@ -128,7 +138,7 @@ protected function generateDifferentSize($id, $imageDir, $belongsTo) $imageTypes = ImageType::getImagesTypes($belongsTo); foreach ($imageTypes as $imageType) { - $resized &= $this->resize($id, $imageDir, $imageType); + $resized &= $this->resize($path, $imageType, $extension); } } catch (PrestaShopException $e) { throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); @@ -144,29 +154,29 @@ protected function generateDifferentSize($id, $imageDir, $belongsTo) /** * Resizes the image depending from its type * - * @param int $id - * @param string $imageDir + * @param string $path * @param array $imageType + * @param string $extension * * @return bool */ - private function resize($id, $imageDir, array $imageType) + private function resize(string $path, array $imageType, string $extension) { - $ext = '.jpg'; $width = $imageType['width']; $height = $imageType['height']; if (Configuration::get('PS_HIGHT_DPI')) { - $ext = '2x.jpg'; + $extension = '2x.' . $extension; $width *= 2; $height *= 2; } return ImageManager::resize( - $imageDir . $id . '.jpg', - $imageDir . $id . '-' . stripslashes($imageType['name']) . $ext, + $path . '.' . $extension, + $path . '-' . stripslashes($imageType['name']) . '.' . $extension, (int) $width, - (int) $height + (int) $height, + $extension ); } } From ee3d814ec36521ee9fe4038b9ffa32b5d9800fea Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 16:30:26 +0300 Subject: [PATCH 07/87] add interface for ProductImageUploader. Use repository to load image --- .../Exception/CannotUnlinkImageException.php | 35 ++++++++++ .../Image/Uploader/ProductImageUploader.php | 64 ++++++++----------- .../{Image => }/ProductImagePathFactory.php | 2 +- .../Repository/ProductImageRepository.php | 60 +++++++++++++++++ .../ProductImageNotFoundException.php | 36 +++++++++++ .../Image/ProductImageUploaderInterface.php | 38 +++++++++++ 6 files changed, 197 insertions(+), 38 deletions(-) create mode 100644 src/Adapter/Image/Exception/CannotUnlinkImageException.php rename src/Adapter/Product/{Image => }/ProductImagePathFactory.php (97%) create mode 100644 src/Adapter/Product/Repository/ProductImageRepository.php create mode 100644 src/Core/Domain/Product/Image/Exception/ProductImageNotFoundException.php create mode 100644 src/Core/Domain/Product/Image/ProductImageUploaderInterface.php diff --git a/src/Adapter/Image/Exception/CannotUnlinkImageException.php b/src/Adapter/Image/Exception/CannotUnlinkImageException.php new file mode 100644 index 0000000000000..6c4f28bfc2ceb --- /dev/null +++ b/src/Adapter/Image/Exception/CannotUnlinkImageException.php @@ -0,0 +1,35 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Image\Exception; + +use PrestaShop\PrestaShop\Core\Exception\CoreException; + +class CannotUnlinkImageException extends CoreException +{ +} diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 80803ece18d8a..21ec8221baf85 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -32,15 +32,18 @@ use Hook; use Image; use ImageManager; -use PrestaShop\PrestaShop\Adapter\Product\Image\ProductImagePathFactory; +use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; +use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; -use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\CannotUnlinkImageException; -use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ImageNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ProductImageUploaderInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; +use PrestaShop\PrestaShop\Core\Exception\CoreException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; +use PrestaShopException; -final class ProductImageUploader extends AbstractImageUploader +final class ProductImageUploader extends AbstractImageUploader implements ProductImageUploaderInterface { /** * @var UploadSizeConfigurationInterface @@ -62,44 +65,49 @@ final class ProductImageUploader extends AbstractImageUploader */ private $contextShopId; + /** + * @var ProductImageRepository + */ + private $productImageRepository; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param array $contextShopIdsList * @param int $contextShopId + * @param ProductImageRepository $productImageRepository */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, array $contextShopIdsList, - int $contextShopId + int $contextShopId, + ProductImageRepository $productImageRepository ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopIdsList = $contextShopIdsList; $this->contextShopId = $contextShopId; + $this->productImageRepository = $productImageRepository; } /** * {@inheritdoc} */ - public function upload( - Image $image, - string $filePath, - string $format - ): void { + public function upload(ImageId $imageId, string $filePath): void + { + $image = $this->productImageRepository->get($imageId); $this->checkMemory($filePath); $this->productImagePathFactory->createDestinationDirectory($image); $this->copyToDestination($filePath, $image); $this->generateDifferentSizeImages( $this->productImagePathFactory->getBasePath($image, false), - 'products', - $format + 'products' ); Hook::exec('actionWatermark', ['id_image' => $image->id, 'id_product' => $image->id_product]); - //@Todo: wait for multishop specs + //@Todo: double-check multishop $image->associateTo($this->contextShopIdsList); $this->deleteOldGeneratedImages($image); } @@ -142,8 +150,12 @@ private function deleteOldGeneratedImages(Image $image): void */ private function copyToDestination(string $tmpImageName, Image $image) { - if (!ImageManager::resize($tmpImageName, $this->productImagePathFactory->getBasePath($image, true))) { - throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); + try { + if (!ImageManager::resize($tmpImageName, $this->productImagePathFactory->getBasePath($image, true))) { + throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); + } + } catch (PrestaShopException $e) { + throw new CoreException('Error occurred when trying to resize product images', 0, $e); } } @@ -160,26 +172,4 @@ private function checkMemory(string $tmpImageName): void throw new MemoryLimitException('Due to memory limit restrictions, this image cannot be loaded. Increase your memory_limit value.'); } } - - /** - * @param ImageId $imageId - * - * @return Image - * - * @throws ImageNotFoundException - */ - private function loadImageEntity(ImageId $imageId): Image - { - $imageIdValue = $imageId->getValue(); - $image = new Image($imageIdValue); - - if ((int) $image->id !== $imageIdValue) { - throw new ImageNotFoundException(sprintf( - 'Image entity with id #%s does not exist', - $imageIdValue - )); - } - - return $image; - } } diff --git a/src/Adapter/Product/Image/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php similarity index 97% rename from src/Adapter/Product/Image/ProductImagePathFactory.php rename to src/Adapter/Product/ProductImagePathFactory.php index c6a943b5d761e..564547497e896 100644 --- a/src/Adapter/Product/Image/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -26,7 +26,7 @@ declare(strict_types=1); -namespace PrestaShop\PrestaShop\Adapter\Product\Image; +namespace PrestaShop\PrestaShop\Adapter\Product; use Image; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php new file mode 100644 index 0000000000000..7284a3a4d8f1b --- /dev/null +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -0,0 +1,60 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Product\Repository; + +use Image; +use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; +use PrestaShop\PrestaShop\Core\Exception\CoreException; + +/** + * Provides access to product Image data source + */ +class ProductImageRepository extends AbstractObjectModelRepository +{ + /** + * @param ImageId $imageId + * + * @return Image + * + * @throws CoreException + */ + public function get(ImageId $imageId): Image + { + /** @var Image $image */ + $image = $this->getObjectModel( + $imageId->getValue(), + Image::class, + ProductImageNotFoundException::class + ); + + return $image; + } +} diff --git a/src/Core/Domain/Product/Image/Exception/ProductImageNotFoundException.php b/src/Core/Domain/Product/Image/Exception/ProductImageNotFoundException.php new file mode 100644 index 0000000000000..5dc468b727f97 --- /dev/null +++ b/src/Core/Domain/Product/Image/Exception/ProductImageNotFoundException.php @@ -0,0 +1,36 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception; + +/** + * Is thrown when product image is not found + */ +class ProductImageNotFoundException extends ProductImageException +{ +} diff --git a/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php b/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php new file mode 100644 index 0000000000000..a94a61579bb35 --- /dev/null +++ b/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php @@ -0,0 +1,38 @@ + + * @copyright 2007-2020 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + * International Registered Trademark & Property of PrestaShop SA + */ + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image; + +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; + +interface ProductImageUploaderInterface +{ + /** + * @param ImageId $imageId + * @param string $filePath + */ + public function upload(ImageId $imageId, string $filePath): void; +} From 5fe5f5a181497396e88dddbece0ef67bd529bec2 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 17:18:09 +0300 Subject: [PATCH 08/87] reuse methods from AbstractUploader --- .../Image/Uploader/ProductImageUploader.php | 54 +++---------------- 1 file changed, 8 insertions(+), 46 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 21ec8221baf85..ab68feac1e808 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -31,17 +31,12 @@ use ErrorException; use Hook; use Image; -use ImageManager; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ProductImageUploaderInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; -use PrestaShop\PrestaShop\Core\Exception\CoreException; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; -use PrestaShopException; final class ProductImageUploader extends AbstractImageUploader implements ProductImageUploaderInterface { @@ -73,22 +68,23 @@ final class ProductImageUploader extends AbstractImageUploader implements Produc /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory + * @param ProductImageRepository $productImageRepository * @param array $contextShopIdsList * @param int $contextShopId - * @param ProductImageRepository $productImageRepository */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, + ProductImageRepository $productImageRepository, + //@todo; how does context have shopIds & singe shopId ? Maybe we can clarify & harmonize it in MultistoreContextChecker? array $contextShopIdsList, - int $contextShopId, - ProductImageRepository $productImageRepository + int $contextShopId ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; + $this->productImageRepository = $productImageRepository; $this->contextShopIdsList = $contextShopIdsList; $this->contextShopId = $contextShopId; - $this->productImageRepository = $productImageRepository; } /** @@ -97,14 +93,11 @@ public function __construct( public function upload(ImageId $imageId, string $filePath): void { $image = $this->productImageRepository->get($imageId); - $this->checkMemory($filePath); $this->productImagePathFactory->createDestinationDirectory($image); - $this->copyToDestination($filePath, $image); - $this->generateDifferentSizeImages( - $this->productImagePathFactory->getBasePath($image, false), - 'products' - ); + //@todo: this will unlink the image. Can we trust that the $filePath is in temp? + $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); + $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); Hook::exec('actionWatermark', ['id_image' => $image->id, 'id_product' => $image->id_product]); //@Todo: double-check multishop @@ -141,35 +134,4 @@ private function deleteOldGeneratedImages(Image $image): void } } } - - /** - * @param string $tmpImageName - * @param Image $image - * - * @throws ImageOptimizationException - */ - private function copyToDestination(string $tmpImageName, Image $image) - { - try { - if (!ImageManager::resize($tmpImageName, $this->productImagePathFactory->getBasePath($image, true))) { - throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); - } - } catch (PrestaShopException $e) { - throw new CoreException('Error occurred when trying to resize product images', 0, $e); - } - } - - /** - * Evaluate the memory required to resize the image: if it's too much, you can't resize it. - * - * @param string $tmpImageName - * - * @throws MemoryLimitException - */ - private function checkMemory(string $tmpImageName): void - { - if (!ImageManager::checkImageMemoryLimit($tmpImageName)) { - throw new MemoryLimitException('Due to memory limit restrictions, this image cannot be loaded. Increase your memory_limit value.'); - } - } } From ed57699fbf47589559dd2404ae8d12d18efb3e89 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 19 Oct 2020 17:35:17 +0300 Subject: [PATCH 09/87] add some todo`s & register services --- src/Adapter/Image/Uploader/AbstractImageUploader.php | 1 + src/Adapter/Image/Uploader/ProductImageUploader.php | 10 +++++----- .../Resources/config/services/adapter/image.yml | 9 +++++++++ .../Resources/config/services/adapter/product.yml | 8 ++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index a68ee2cba9098..44a96e7ef0e12 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -106,6 +106,7 @@ protected function uploadFromTemp($temporaryImageName, $destination) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } + //@todo: should we care about tempfile deletion? shouldn't we leave it for garbage collector? unlink($temporaryImageName); } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index ab68feac1e808..feb393b242502 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -50,6 +50,11 @@ final class ProductImageUploader extends AbstractImageUploader implements Produc */ private $productImagePathFactory; + /** + * @var ProductImageRepository + */ + private $productImageRepository; + /** * @var array */ @@ -60,11 +65,6 @@ final class ProductImageUploader extends AbstractImageUploader implements Produc */ private $contextShopId; - /** - * @var ProductImageRepository - */ - private $productImageRepository; - /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index b800e6ac337fc..be0076e67cb2c 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -30,3 +30,12 @@ services: prestashop.adapter.image.uploader.supplier_image_uploader: class: 'PrestaShop\PrestaShop\Adapter\Image\Uploader\SupplierImageUploader' + + prestashop.adapter.image.uploader.product_image_uploader: + class: 'PrestaShop\PrestaShop\Adapter\Image\Uploader\ProductImageUploader' + arguments: + - '@prestashop.core.configuration.upload_size_configuration' + - '@prestashop.adapter.product.product_image_path_factory' + - '@prestashop.adapter.product.repository.product_image_repository' + - "@=service('prestashop.adapter.shop.context').getContextListShopID()" + - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index dfbb8f1c1c6d4..779e7d1e065e4 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -415,3 +415,11 @@ services: tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand + + prestashop.adapter.product.repository.product_image_repository: + class: PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository + + prestashop.adapter.product.product_image_path_factory: + class: PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory + arguments: + - "@=service('prestashop.adapter.legacy.configuration').getBoolean('PS_LEGACY_IMAGES')" From 1cadd69f45aeeb35603dd44a134820922bc3b996 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 10:50:54 +0300 Subject: [PATCH 10/87] reuse abstract class method in manufacturerImageUploader --- .../Uploader/ManufacturerImageUploader.php | 57 +------------------ 1 file changed, 1 insertion(+), 56 deletions(-) diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index 84ad2680114f4..9fa6b6e65ae54 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -26,14 +26,11 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; -use Configuration; use ImageManager; -use ImageType; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; -use PrestaShopException; use Symfony\Component\HttpFoundation\File\UploadedFile; /** @@ -66,58 +63,6 @@ public function upload($manufacturerId, UploadedFile $image) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } - $this->generateDifferentSizeImages($manufacturerId); - } - - /** - * @param int $manufacturerId - * - * @return bool - */ - private function generateDifferentSizeImages($manufacturerId) - { - $resized = true; - $generateHighDpiImages = (bool) Configuration::get('PS_HIGHT_DPI'); - - try { - /* Generate images with different size */ - if (count($_FILES) && - file_exists(_PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg') - ) { - $imageTypes = ImageType::getImagesTypes('manufacturers'); - - foreach ($imageTypes as $imageType) { - $resized &= ImageManager::resize( - _PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg', - _PS_MANU_IMG_DIR_ . $manufacturerId . '-' . stripslashes($imageType['name']) . '.jpg', - (int) $imageType['width'], - (int) $imageType['height'] - ); - - if ($generateHighDpiImages) { - $resized &= ImageManager::resize( - _PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg', - _PS_MANU_IMG_DIR_ . $manufacturerId . '-' . stripslashes($imageType['name']) . '2x.jpg', - (int) $imageType['width'] * 2, - (int) $imageType['height'] * 2 - ); - } - } - - $currentLogo = _PS_TMP_IMG_DIR_ . 'manufacturer_mini_' . $manufacturerId . '.jpg'; - - if ($resized && file_exists($currentLogo)) { - unlink($currentLogo); - } - } - } catch (PrestaShopException $e) { - throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); - } - - if (!$resized) { - throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); - } - - return $resized; + $this->generateDifferentSizeImages(_PS_MANU_IMG_DIR_ . $manufacturerId, 'manufacturers'); } } From 50f77b055ad50d086b3255870911662520a92db5 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 13:14:34 +0300 Subject: [PATCH 11/87] add get cached images paths --- src/Adapter/Image/Uploader/ProductImageUploader.php | 7 +++++-- src/Adapter/Product/ProductImagePathFactory.php | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index feb393b242502..7f3e735549d54 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -112,9 +112,11 @@ public function upload(ImageId $imageId, string $filePath): void */ private function deleteOldGeneratedImages(Image $image): void { + $productId = (int) $image->id_product; + $oldGeneratedImages = [ - _PS_TMP_IMG_DIR_ . 'product_' . (int) $image->id . '.jpg', - _PS_TMP_IMG_DIR_ . 'product_mini_' . (int) $image->id_product . '_' . $this->contextShopId . '.jpg', + $this->productImagePathFactory->getCachedCover($productId, $this->contextShopId), + $this->productImagePathFactory->getCachedThumbnail($productId), ]; foreach ($oldGeneratedImages as $oldImage) { @@ -122,6 +124,7 @@ private function deleteOldGeneratedImages(Image $image): void try { unlink($oldImage); } catch (ErrorException $e) { + //@todo: do we really need to fail on cached images deletion? It was suppressed in legacy throw new CannotUnlinkImageException( sprintf( 'Failed to remove old generated image "%s"', diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index 564547497e896..94eb58086878b 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -74,4 +74,15 @@ public function createDestinationDirectory(Image $image): void $image->id_product )); } + + //@todo: make static? + public function getCachedCover(int $productId, int $shopId): string + { + return sprintf('%sproduct_mini_%s_%s.jpg', _PS_TMP_IMG_DIR_, $productId, $shopId); + } + + public function getCachedThumbnail(int $productId): string + { + return sprintf('%sproduct_%s.jpg', _PS_TMP_IMG_DIR_, $productId); + } } From 9af9b1e9d8e426d8a839b86545143a7ccc9d07e7 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 13:37:35 +0300 Subject: [PATCH 12/87] remove interface for uploader. Add repository create method. Implement handler; --- .../Image/Uploader/ProductImageUploader.php | 31 +++------------ .../CommandHandler/AddProductImageHandler.php | 39 ++++++++++++++++++- .../Repository/ProductImageRepository.php | 30 ++++++++++++++ .../Image/ProductImageUploaderInterface.php | 38 ------------------ .../config/services/adapter/image.yml | 2 - .../config/services/adapter/product.yml | 4 ++ 6 files changed, 77 insertions(+), 67 deletions(-) delete mode 100644 src/Core/Domain/Product/Image/ProductImageUploaderInterface.php diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 7f3e735549d54..6fbe173eccf70 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -33,12 +33,10 @@ use Image; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; -use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; -use PrestaShop\PrestaShop\Core\Domain\Product\Image\ProductImageUploaderInterface; -use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; -final class ProductImageUploader extends AbstractImageUploader implements ProductImageUploaderInterface +//@todo: do we really need an interface? depends if we use it in controller or in command handler +class ProductImageUploader extends AbstractImageUploader { /** * @var UploadSizeConfigurationInterface @@ -50,16 +48,6 @@ final class ProductImageUploader extends AbstractImageUploader implements Produc */ private $productImagePathFactory; - /** - * @var ProductImageRepository - */ - private $productImageRepository; - - /** - * @var array - */ - private $contextShopIdsList; - /** * @var int */ @@ -68,40 +56,31 @@ final class ProductImageUploader extends AbstractImageUploader implements Produc /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory - * @param ProductImageRepository $productImageRepository - * @param array $contextShopIdsList * @param int $contextShopId */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, - ProductImageRepository $productImageRepository, - //@todo; how does context have shopIds & singe shopId ? Maybe we can clarify & harmonize it in MultistoreContextChecker? - array $contextShopIdsList, int $contextShopId ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; - $this->productImageRepository = $productImageRepository; - $this->contextShopIdsList = $contextShopIdsList; $this->contextShopId = $contextShopId; } /** * {@inheritdoc} */ - public function upload(ImageId $imageId, string $filePath): void + public function upload(Image $image, string $filePath): void { - $image = $this->productImageRepository->get($imageId); $this->productImagePathFactory->createDestinationDirectory($image); //@todo: this will unlink the image. Can we trust that the $filePath is in temp? $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); - Hook::exec('actionWatermark', ['id_image' => $image->id, 'id_product' => $image->id_product]); - //@Todo: double-check multishop - $image->associateTo($this->contextShopIdsList); + Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); + //@todo: moved multishop association from here to Repository when Image objModel is created $this->deleteOldGeneratedImages($image); } diff --git a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php index f9ed5b7d89bed..129dc33332b1b 100644 --- a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php +++ b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php @@ -28,6 +28,8 @@ namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; +use PrestaShop\PrestaShop\Adapter\Image\Uploader\ProductImageUploader; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; use PrestaShop\PrestaShop\Core\Domain\Product\CommandHandler\AddProductImageHandlerInterface; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; @@ -37,11 +39,46 @@ */ final class AddProductImageHandler implements AddProductImageHandlerInterface { + /** + * @var ProductImageUploader + */ + private $productImageUploader; + + /** + * @var ProductImageRepository + */ + private $productImageRepository; + + /** + * @var array + */ + private $contextShopIds; + + /** + * @param ProductImageUploader $productImageUploader + * @param ProductImageRepository $productImageRepository + * @param array $contextShopIds + */ + public function __construct( + ProductImageUploader $productImageUploader, + ProductImageRepository $productImageRepository, + array $contextShopIds + ) { + $this->productImageUploader = $productImageUploader; + $this->productImageRepository = $productImageRepository; + $this->contextShopIds = $contextShopIds; + } + /** * {@inheritdoc} */ public function handle(AddProductImageCommand $command): ImageId { - // TODO: Implement handle() method. + $image = $this->productImageRepository->create($command->getProductId(), $this->contextShopIds); + + //@todo: db transaction rollback if upload fails? + $this->productImageUploader->upload($image, $command->getPathName()); + + return new ImageId((int) $image->id); } } diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php index 7284a3a4d8f1b..a35207e84bd18 100644 --- a/src/Adapter/Product/Repository/ProductImageRepository.php +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -32,6 +32,7 @@ use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageNotFoundException; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; +use PrestaShop\PrestaShop\Core\Domain\Product\ValueObject\ProductId; use PrestaShop\PrestaShop\Core\Exception\CoreException; /** @@ -39,6 +40,35 @@ */ class ProductImageRepository extends AbstractObjectModelRepository { + /** + * @param ProductId $productId + * @param int[] $shopIds + * + * @return Image + * + * @throws \PrestaShopDatabaseException + * @throws \PrestaShopException + */ + public function create(ProductId $productId, array $shopIds): Image + { + $productIdValue = $productId->getValue(); + $image = new \Image(); + $image->id_product = $productIdValue; + $image->position = Image::getHighestPosition($productIdValue) + 1; + + if (!Image::getCover($productIdValue)) { + $image->cover = 1; + } else { + $image->cover = 0; + } + + //@todo: wrap in try catch + $image->add(); + $image->associateTo($shopIds); + + return $image; + } + /** * @param ImageId $imageId * diff --git a/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php b/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php deleted file mode 100644 index a94a61579bb35..0000000000000 --- a/src/Core/Domain/Product/Image/ProductImageUploaderInterface.php +++ /dev/null @@ -1,38 +0,0 @@ - - * @copyright 2007-2020 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) - * International Registered Trademark & Property of PrestaShop SA - */ - -namespace PrestaShop\PrestaShop\Core\Domain\Product\Image; - -use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; - -interface ProductImageUploaderInterface -{ - /** - * @param ImageId $imageId - * @param string $filePath - */ - public function upload(ImageId $imageId, string $filePath): void; -} diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index be0076e67cb2c..eefd5c1c67c77 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -36,6 +36,4 @@ services: arguments: - '@prestashop.core.configuration.upload_size_configuration' - '@prestashop.adapter.product.product_image_path_factory' - - '@prestashop.adapter.product.repository.product_image_repository' - - "@=service('prestashop.adapter.shop.context').getContextListShopID()" - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 779e7d1e065e4..1a748980ac179 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -412,6 +412,10 @@ services: prestashop.adapter.product.command_handler.add_product_image_handler: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\AddProductImageHandler + arguments: + - '@prestashop.adapter.image.uploader.product_image_uploader' + - '@prestashop.adapter.product.repository.product_image_repository' + - "@=service('prestashop.adapter.shop.context').getContextListShopID()" tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand From 70cb1b701db1a7f72e293bc009c7e52021888015 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 14:06:54 +0300 Subject: [PATCH 13/87] define TEST img dir --- config/defines.inc.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/defines.inc.php b/config/defines.inc.php index 37ca970221db1..c0aa450be1e5b 100755 --- a/config/defines.inc.php +++ b/config/defines.inc.php @@ -135,8 +135,10 @@ define('_PS_TCPDF_PATH_', _PS_TOOL_DIR_.'tcpdf/'); if (!defined('_PS_IMG_DIR_')) { - define('_PS_IMG_DIR_', _PS_ROOT_DIR_.'/img/'); + $dir = (defined('_PS_IN_TEST_') && _PS_IN_TEST_) ? '/tests/Resources/img/' : '/img/'; + define('_PS_IMG_DIR_', _PS_ROOT_DIR_.$dir); } + if (!defined('_PS_HOST_MODE_')) { define('_PS_CORE_IMG_DIR_', _PS_CORE_DIR_.'/img/'); } else { From 18c3e486b90c26f628bcca6edd729ce6d0653646 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 14:59:34 +0300 Subject: [PATCH 14/87] start adding behat for image upload. behats cannot clean test img dir due to permissions --- .../Command/AddProductImageCommand.php | 5 +- .../Features/Context/CommonFeatureContext.php | 17 +++++ .../Product/ProductImageFeatureContext.php | 69 +++++++++++++++++++ .../Scenario/Product/add_image.feature | 15 ++++ tests/Integration/Behaviour/behat.yml | 1 + tests/Resources/img/.htaccess | 20 ++++++ tests/Resources/img/index.php | 34 +++++++++ 7 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php create mode 100644 tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature create mode 100644 tests/Resources/img/.htaccess create mode 100644 tests/Resources/img/index.php diff --git a/src/Core/Domain/Product/Command/AddProductImageCommand.php b/src/Core/Domain/Product/Command/AddProductImageCommand.php index b304486f745ce..9cff94acfc952 100644 --- a/src/Core/Domain/Product/Command/AddProductImageCommand.php +++ b/src/Core/Domain/Product/Command/AddProductImageCommand.php @@ -68,13 +68,14 @@ class AddProductImageCommand * @param string $mimeType */ public function __construct( - ProductId $productId, + int $productId, string $originalName, string $pathName, int $fileSize, string $mimeType ) { - $this->productId = $productId; + $this->productId = new ProductId($productId); + //@todo: assert file constraints or leave it to uploader? $this->originalName = $originalName; $this->pathName = $pathName; $this->fileSize = $fileSize; diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index eeae4df628c17..99a81752c98ff 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -123,6 +123,23 @@ public static function clearDownloads(): void } } + /** + * @AfterFeature @clear-img-after-feature + */ + public static function clearImgDir(): void + { + $filesToSkip = [ + _PS_IMG_DIR_ . 'index.php', + _PS_IMG_DIR_ . '.htaccess', + ]; + + foreach (glob(_PS_IMG_DIR_ . '*') as $file) { + if (is_file($file) && !in_array($file, $filesToSkip)) { + unlink($file); + } + } + } + /** * @AfterFeature @clear-cache-after-feature */ diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php new file mode 100644 index 0000000000000..559c44ff8c401 --- /dev/null +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -0,0 +1,69 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace Tests\Integration\Behaviour\Features\Context\Domain\Product; + +use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; +use RuntimeException; + +class ProductImageFeatureContext extends AbstractProductFeatureContext +{ + /** + * @When I add new product :productReference image :fileName + * + * @param string $productReference + * @param string $fileName + */ + public function uploadImage(string $productReference, string $fileName) + { + $pathName = $this->uploadDummyFile($fileName); + + $this->getCommandBus()->handle(new AddProductImageCommand( + $this->getSharedStorage()->get($productReference), + $fileName, + $pathName, + filesize($pathName), + mime_content_type($pathName) + )); + } + + //@todo: homogenize with attachments context method + private function uploadDummyFile(string $fileName): string + { + $source = _PS_ROOT_DIR_ . '/tests/Resources/dummyFile/' . $fileName; + + if (!is_file($source)) { + throw new RuntimeException('%s is not a file', $source); + } + + $destination = tempnam(sys_get_temp_dir(), 'PS_TEST_'); + copy($source, $destination); + + return $destination; + } +} diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature new file mode 100644 index 0000000000000..121d29b4d1605 --- /dev/null +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -0,0 +1,15 @@ +# ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s product --tags add-image +@reset-database-before-feature +@clear-cache-after-feature +@clear-img-after-feature +@add-image +Feature: Add product image from Back Office (BO) + As an employee I need to be able to add new product image + + Scenario: Add new product image + Given I add product "product1" with following information: + | name | en-US:bottle of beer | + | is_virtual | false | + And product "product1" type should be standard +#todo And product "product1" should have no images + When I add new product "product1" image "app_icon.png" diff --git a/tests/Integration/Behaviour/behat.yml b/tests/Integration/Behaviour/behat.yml index 15802b2241bc6..8b2d745117247 100644 --- a/tests/Integration/Behaviour/behat.yml +++ b/tests/Integration/Behaviour/behat.yml @@ -244,3 +244,4 @@ default: - Tests\Integration\Behaviour\Features\Context\Domain\Product\DuplicateProductFeatureContext - Tests\Integration\Behaviour\Features\Transform\StringToBoolTransformContext - Tests\Integration\Behaviour\Features\Transform\StringToArrayTransformContext + - Tests\Integration\Behaviour\Features\Context\Domain\Product\ProductImageFeatureContext diff --git a/tests/Resources/img/.htaccess b/tests/Resources/img/.htaccess new file mode 100644 index 0000000000000..124358fc9754d --- /dev/null +++ b/tests/Resources/img/.htaccess @@ -0,0 +1,20 @@ + + php_flag engine off + + +# Apache 2.2 + + Order deny,allow + Deny from all + + Allow from all + + + +# Apache 2.4 + + Require all denied + + Require all granted + + diff --git a/tests/Resources/img/index.php b/tests/Resources/img/index.php new file mode 100644 index 0000000000000..c7835bc882bbf --- /dev/null +++ b/tests/Resources/img/index.php @@ -0,0 +1,34 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ +header('Expires: Mon, 26 Jul 1997 05:00:00 GMT'); +header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT'); + +header('Cache-Control: no-store, no-cache, must-revalidate'); +header('Cache-Control: post-check=0, pre-check=0', false); +header('Pragma: no-cache'); + +header('Location: ../'); +exit; From 9f4a748d2b067c6eb0abae93ce378b82c33679a1 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 16:16:48 +0300 Subject: [PATCH 15/87] remove unused arguments. Add todos --- .../Image/Uploader/ProductImageUploader.php | 4 ++-- .../Command/AddProductImageCommand.php | 20 ++----------------- .../Product/ProductImageFeatureContext.php | 5 ++--- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 6fbe173eccf70..40ff035a958b7 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -81,7 +81,7 @@ public function upload(Image $image, string $filePath): void Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); //@todo: moved multishop association from here to Repository when Image objModel is created - $this->deleteOldGeneratedImages($image); + $this->deleteCachedImages($image); } /** @@ -89,7 +89,7 @@ public function upload(Image $image, string $filePath): void * * @throws CannotUnlinkImageException */ - private function deleteOldGeneratedImages(Image $image): void + private function deleteCachedImages(Image $image): void { $productId = (int) $image->id_product; diff --git a/src/Core/Domain/Product/Command/AddProductImageCommand.php b/src/Core/Domain/Product/Command/AddProductImageCommand.php index 9cff94acfc952..b185259c7fe5d 100644 --- a/src/Core/Domain/Product/Command/AddProductImageCommand.php +++ b/src/Core/Domain/Product/Command/AddProductImageCommand.php @@ -51,35 +51,19 @@ class AddProductImageCommand private $pathName; /** - * @var int - */ - private $fileSize; - - /** - * @var string - */ - private $mimeType; - - /** - * @param ProductId $productId + * @param int $productId * @param string $originalName * @param string $pathName - * @param int $fileSize - * @param string $mimeType */ public function __construct( int $productId, string $originalName, - string $pathName, - int $fileSize, - string $mimeType + string $pathName ) { $this->productId = new ProductId($productId); //@todo: assert file constraints or leave it to uploader? $this->originalName = $originalName; $this->pathName = $pathName; - $this->fileSize = $fileSize; - $this->mimeType = $mimeType; } /** diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 559c44ff8c401..108a38e8f9523 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -41,14 +41,13 @@ class ProductImageFeatureContext extends AbstractProductFeatureContext */ public function uploadImage(string $productReference, string $fileName) { + //@todo: behats database contains empty ImageType. $pathName = $this->uploadDummyFile($fileName); $this->getCommandBus()->handle(new AddProductImageCommand( $this->getSharedStorage()->get($productReference), $fileName, - $pathName, - filesize($pathName), - mime_content_type($pathName) + $pathName )); } From 54602ddf80fd125f3de5a2f7e0fb0dc925a216b5 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 16:31:25 +0300 Subject: [PATCH 16/87] rename var --- src/Adapter/Image/Uploader/ProductImageUploader.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 40ff035a958b7..54607f4753b05 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -93,21 +93,21 @@ private function deleteCachedImages(Image $image): void { $productId = (int) $image->id_product; - $oldGeneratedImages = [ + $cachedImages = [ $this->productImagePathFactory->getCachedCover($productId, $this->contextShopId), $this->productImagePathFactory->getCachedThumbnail($productId), ]; - foreach ($oldGeneratedImages as $oldImage) { - if (file_exists($oldImage)) { + foreach ($cachedImages as $cachedImage) { + if (file_exists($cachedImage)) { try { - unlink($oldImage); + unlink($cachedImage); } catch (ErrorException $e) { //@todo: do we really need to fail on cached images deletion? It was suppressed in legacy throw new CannotUnlinkImageException( sprintf( - 'Failed to remove old generated image "%s"', - $oldImage + 'Failed to remove cached image "%s"', + $cachedImage ), 0, $e From 4dc2575e3967dcaac080fb6d75d681415ad47ea2 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 20 Oct 2020 16:34:22 +0300 Subject: [PATCH 17/87] one more todo --- .../Behaviour/Features/Context/CommonFeatureContext.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index 99a81752c98ff..6bdba16e9f58e 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -128,6 +128,7 @@ public static function clearDownloads(): void */ public static function clearImgDir(): void { + //@todo: its not working because img/* directories permissions. Those are created with Image->createImgFolder() $filesToSkip = [ _PS_IMG_DIR_ . 'index.php', _PS_IMG_DIR_ . '.htaccess', From fc76ffa8533fe0a22449dd83d4c35a007739ce03 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 21 Oct 2020 18:04:15 +0300 Subject: [PATCH 18/87] remove unused index.php and .htaccess files. Fix recursive img cleaning dir --- .../Features/Context/CommonFeatureContext.php | 41 ++++++++++++------- tests/Resources/download/.gitkeep | 0 tests/Resources/download/.htaccess | 10 ----- tests/Resources/download/index.php | 34 --------------- tests/Resources/img/.gitkeep | 0 tests/Resources/img/.htaccess | 20 --------- tests/Resources/img/index.php | 34 --------------- 7 files changed, 26 insertions(+), 113 deletions(-) create mode 100644 tests/Resources/download/.gitkeep delete mode 100644 tests/Resources/download/.htaccess delete mode 100644 tests/Resources/download/index.php create mode 100644 tests/Resources/img/.gitkeep delete mode 100644 tests/Resources/img/.htaccess delete mode 100644 tests/Resources/img/index.php diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index 6bdba16e9f58e..e99c75f094e60 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -111,13 +111,8 @@ public static function getContainer() */ public static function clearDownloads(): void { - $filesToSkip = [ - _PS_DOWNLOAD_DIR_ . 'index.php', - _PS_DOWNLOAD_DIR_ . '.htaccess', - ]; - foreach (glob(_PS_DOWNLOAD_DIR_ . '*') as $file) { - if (is_file($file) && !in_array($file, $filesToSkip)) { + if (is_file($file)) { unlink($file); } } @@ -128,17 +123,33 @@ public static function clearDownloads(): void */ public static function clearImgDir(): void { - //@todo: its not working because img/* directories permissions. Those are created with Image->createImgFolder() - $filesToSkip = [ - _PS_IMG_DIR_ . 'index.php', - _PS_IMG_DIR_ . '.htaccess', - ]; - - foreach (glob(_PS_IMG_DIR_ . '*') as $file) { - if (is_file($file) && !in_array($file, $filesToSkip)) { - unlink($file); + function unlinkFilesRecursively(string $recursiveDir = null): void + { + $startingDir = _PS_IMG_DIR_; + if ($recursiveDir) { + $startingDir = $recursiveDir . '/'; + } + + $dirs = []; + foreach (glob($startingDir . '*') as $file) { + if (is_dir($file)) { + $dirs[] = $file; + + continue; + } + + if (is_file($file)) { + unlink($file); + } + } + + foreach ($dirs as $dir) { + unlinkFilesRecursively($dir); + rmdir($dir); } } + + unlinkFilesRecursively(); } /** diff --git a/tests/Resources/download/.gitkeep b/tests/Resources/download/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/Resources/download/.htaccess b/tests/Resources/download/.htaccess deleted file mode 100644 index 3de9e4008bd96..0000000000000 --- a/tests/Resources/download/.htaccess +++ /dev/null @@ -1,10 +0,0 @@ -# Apache 2.2 - - Order deny,allow - Deny from all - - -# Apache 2.4 - - Require all denied - diff --git a/tests/Resources/download/index.php b/tests/Resources/download/index.php deleted file mode 100644 index c7835bc882bbf..0000000000000 --- a/tests/Resources/download/index.php +++ /dev/null @@ -1,34 +0,0 @@ - - * @copyright Since 2007 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) - */ -header('Expires: Mon, 26 Jul 1997 05:00:00 GMT'); -header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT'); - -header('Cache-Control: no-store, no-cache, must-revalidate'); -header('Cache-Control: post-check=0, pre-check=0', false); -header('Pragma: no-cache'); - -header('Location: ../'); -exit; diff --git a/tests/Resources/img/.gitkeep b/tests/Resources/img/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/Resources/img/.htaccess b/tests/Resources/img/.htaccess deleted file mode 100644 index 124358fc9754d..0000000000000 --- a/tests/Resources/img/.htaccess +++ /dev/null @@ -1,20 +0,0 @@ - - php_flag engine off - - -# Apache 2.2 - - Order deny,allow - Deny from all - - Allow from all - - - -# Apache 2.4 - - Require all denied - - Require all granted - - diff --git a/tests/Resources/img/index.php b/tests/Resources/img/index.php deleted file mode 100644 index c7835bc882bbf..0000000000000 --- a/tests/Resources/img/index.php +++ /dev/null @@ -1,34 +0,0 @@ - - * @copyright Since 2007 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) - */ -header('Expires: Mon, 26 Jul 1997 05:00:00 GMT'); -header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT'); - -header('Cache-Control: no-store, no-cache, must-revalidate'); -header('Cache-Control: post-check=0, pre-check=0', false); -header('Pragma: no-cache'); - -header('Location: ../'); -exit; From 60668ed6c0dca01d81160a71739a2b5f9ab2f9e7 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 22 Oct 2020 10:24:02 +0300 Subject: [PATCH 19/87] implement interface for uploader. Homogenize it with other uploaders --- .../Image/Uploader/AbstractImageUploader.php | 1 - .../Image/Uploader/ProductImageUploader.php | 28 +++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index 44a96e7ef0e12..a68ee2cba9098 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -106,7 +106,6 @@ protected function uploadFromTemp($temporaryImageName, $destination) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } - //@todo: should we care about tempfile deletion? shouldn't we leave it for garbage collector? unlink($temporaryImageName); } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 54607f4753b05..fe2aba6eb8298 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -33,10 +33,13 @@ use Image; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; +use Symfony\Component\HttpFoundation\File\UploadedFile; -//@todo: do we really need an interface? depends if we use it in controller or in command handler -class ProductImageUploader extends AbstractImageUploader +final class ProductImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * @var UploadSizeConfigurationInterface @@ -53,34 +56,44 @@ class ProductImageUploader extends AbstractImageUploader */ private $contextShopId; + /** + * @var ProductImageRepository + */ + private $productImageRepository; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId + * @param ProductImageRepository $productImageRepository */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, - int $contextShopId + int $contextShopId, + ProductImageRepository $productImageRepository ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; + $this->productImageRepository = $productImageRepository; } /** * {@inheritdoc} */ - public function upload(Image $image, string $filePath): void + public function upload($imageId, UploadedFile $uploadedFile): void { + $this->checkImageIsAllowedForUpload($uploadedFile); + $tmpPath = $this->createTemporaryImage($uploadedFile); + + $image = $this->productImageRepository->get(new ImageId($imageId)); $this->productImagePathFactory->createDestinationDirectory($image); - //@todo: this will unlink the image. Can we trust that the $filePath is in temp? - $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); + $this->uploadFromTemp($tmpPath, $this->productImagePathFactory->getBasePath($image, true)); $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); - //@todo: moved multishop association from here to Repository when Image objModel is created $this->deleteCachedImages($image); } @@ -103,7 +116,6 @@ private function deleteCachedImages(Image $image): void try { unlink($cachedImage); } catch (ErrorException $e) { - //@todo: do we really need to fail on cached images deletion? It was suppressed in legacy throw new CannotUnlinkImageException( sprintf( 'Failed to remove cached image "%s"', From a9915ed7466350c7a1742645ab7cc0c4b7ca77dc Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 22 Oct 2020 12:16:06 +0300 Subject: [PATCH 20/87] Revert " implement interface for uploader. Homogenize it with other uploaders" This reverts commit 84b2771c330bcec8bf67a526de9e6c3b9d3d75d7. --- .../Image/Uploader/AbstractImageUploader.php | 1 + .../Image/Uploader/ProductImageUploader.php | 28 ++++++------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index a68ee2cba9098..44a96e7ef0e12 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -106,6 +106,7 @@ protected function uploadFromTemp($temporaryImageName, $destination) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } + //@todo: should we care about tempfile deletion? shouldn't we leave it for garbage collector? unlink($temporaryImageName); } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index fe2aba6eb8298..54607f4753b05 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -33,13 +33,10 @@ use Image; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; -use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; -use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; -use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; -use Symfony\Component\HttpFoundation\File\UploadedFile; -final class ProductImageUploader extends AbstractImageUploader implements ImageUploaderInterface +//@todo: do we really need an interface? depends if we use it in controller or in command handler +class ProductImageUploader extends AbstractImageUploader { /** * @var UploadSizeConfigurationInterface @@ -56,44 +53,34 @@ final class ProductImageUploader extends AbstractImageUploader implements ImageU */ private $contextShopId; - /** - * @var ProductImageRepository - */ - private $productImageRepository; - /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId - * @param ProductImageRepository $productImageRepository */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, - int $contextShopId, - ProductImageRepository $productImageRepository + int $contextShopId ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; - $this->productImageRepository = $productImageRepository; } /** * {@inheritdoc} */ - public function upload($imageId, UploadedFile $uploadedFile): void + public function upload(Image $image, string $filePath): void { - $this->checkImageIsAllowedForUpload($uploadedFile); - $tmpPath = $this->createTemporaryImage($uploadedFile); - - $image = $this->productImageRepository->get(new ImageId($imageId)); $this->productImagePathFactory->createDestinationDirectory($image); - $this->uploadFromTemp($tmpPath, $this->productImagePathFactory->getBasePath($image, true)); + //@todo: this will unlink the image. Can we trust that the $filePath is in temp? + $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); + //@todo: moved multishop association from here to Repository when Image objModel is created $this->deleteCachedImages($image); } @@ -116,6 +103,7 @@ private function deleteCachedImages(Image $image): void try { unlink($cachedImage); } catch (ErrorException $e) { + //@todo: do we really need to fail on cached images deletion? It was suppressed in legacy throw new CannotUnlinkImageException( sprintf( 'Failed to remove cached image "%s"', From c338d83829a889cf57899d43e40c82e566ec061a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 22 Oct 2020 13:09:08 +0300 Subject: [PATCH 21/87] remove resovled todos --- src/Adapter/Image/Uploader/AbstractImageUploader.php | 1 - src/Adapter/Image/Uploader/ProductImageUploader.php | 7 +++---- .../Product/CommandHandler/AddProductImageHandler.php | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index 44a96e7ef0e12..a68ee2cba9098 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -106,7 +106,6 @@ protected function uploadFromTemp($temporaryImageName, $destination) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } - //@todo: should we care about tempfile deletion? shouldn't we leave it for garbage collector? unlink($temporaryImageName); } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 54607f4753b05..351fbed5a113a 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -35,7 +35,9 @@ use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; -//@todo: do we really need an interface? depends if we use it in controller or in command handler +/** + * Uploads product image to filesystem + */ class ProductImageUploader extends AbstractImageUploader { /** @@ -75,12 +77,10 @@ public function upload(Image $image, string $filePath): void { $this->productImagePathFactory->createDestinationDirectory($image); - //@todo: this will unlink the image. Can we trust that the $filePath is in temp? $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); - //@todo: moved multishop association from here to Repository when Image objModel is created $this->deleteCachedImages($image); } @@ -103,7 +103,6 @@ private function deleteCachedImages(Image $image): void try { unlink($cachedImage); } catch (ErrorException $e) { - //@todo: do we really need to fail on cached images deletion? It was suppressed in legacy throw new CannotUnlinkImageException( sprintf( 'Failed to remove cached image "%s"', diff --git a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php index 129dc33332b1b..9fbd0953c1ee4 100644 --- a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php +++ b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php @@ -76,7 +76,6 @@ public function handle(AddProductImageCommand $command): ImageId { $image = $this->productImageRepository->create($command->getProductId(), $this->contextShopIds); - //@todo: db transaction rollback if upload fails? $this->productImageUploader->upload($image, $command->getPathName()); return new ImageId((int) $image->id); From 84f8d6dbca714e30bdd320718d830566f286ab64 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 22 Oct 2020 14:30:52 +0300 Subject: [PATCH 22/87] introduce method assertImageIsAllowedForUpload and deprecate another. Remove OriginalName related validation because that name is not used anywhere --- .../Image/Uploader/AbstractImageUploader.php | 25 ++++++++++++++----- .../Image/Uploader/ProductImageUploader.php | 1 + .../CommandHandler/AddProductImageHandler.php | 1 - 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index a68ee2cba9098..b3c37818bcb88 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -45,7 +45,8 @@ abstract class AbstractImageUploader { /** - * Check if image is allowed to be uploaded. + * @deprecated + * @see assertImageIsAllowedForUpload() * * @param UploadedFile $image * @@ -53,16 +54,28 @@ abstract class AbstractImageUploader */ protected function checkImageIsAllowedForUpload(UploadedFile $image) { + $this->assertImageIsAllowedForUpload($image->getPathname()); + } + + /** + * @param string $filePath + * + * @throws ImageUploadException + * @throws UploadedImageConstraintException + */ + protected function assertImageIsAllowedForUpload(string $filePath): void + { + if (!is_file($filePath)) { + throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); + } + $maxFileSize = Tools::getMaxUploadSize(); - if ($maxFileSize > 0 && $image->getSize() > $maxFileSize) { + if ($maxFileSize > 0 && filesize($filePath) > $maxFileSize) { throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $image->getSize()), UploadedImageConstraintException::EXCEEDED_SIZE); } - if (!ImageManager::isRealImage($image->getPathname(), $image->getClientMimeType()) - || !ImageManager::isCorrectImageFileExt($image->getClientOriginalName()) - || preg_match('/\%00/', $image->getClientOriginalName()) // prevent null byte injection - ) { + if (!ImageManager::isRealImage($filePath, mime_content_type($filePath))) { throw new UploadedImageConstraintException(sprintf('Image format "%s", not recognized, allowed formats are: .gif, .jpg, .png', $image->getClientOriginalExtension()), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 351fbed5a113a..b9594a27a9526 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -75,6 +75,7 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { + $this->assertImageIsAllowedForUpload($filePath); $this->productImagePathFactory->createDestinationDirectory($image); $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); diff --git a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php index 9fbd0953c1ee4..c60065a653981 100644 --- a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php +++ b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php @@ -75,7 +75,6 @@ public function __construct( public function handle(AddProductImageCommand $command): ImageId { $image = $this->productImageRepository->create($command->getProductId(), $this->contextShopIds); - $this->productImageUploader->upload($image, $command->getPathName()); return new ImageId((int) $image->id); From 65d3e788ac90231ab968cd973dea36eed4fed488 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 22 Oct 2020 17:44:31 +0300 Subject: [PATCH 23/87] add image Generator and validator. WIP --- src/Adapter/Image/ImageGenerator.php | 104 ++++++++++++++++ src/Adapter/Image/ImageValidator.php | 115 ++++++++++++++++++ .../Image/Uploader/ProductImageUploader.php | 22 +++- .../Product/ProductImagePathFactory.php | 10 +- .../config/services/adapter/image.yml | 4 + 5 files changed, 243 insertions(+), 12 deletions(-) create mode 100644 src/Adapter/Image/ImageGenerator.php create mode 100644 src/Adapter/Image/ImageValidator.php diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php new file mode 100644 index 0000000000000..b8eb3b8c62721 --- /dev/null +++ b/src/Adapter/Image/ImageGenerator.php @@ -0,0 +1,104 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Image; + +use Configuration; +use ImageManager; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; +use PrestaShopException; + +class ImageGenerator +{ + /** + * @param string $imagePath + * @param array $imageTypes + * + * @return bool + * + * @throws ImageOptimizationException + * @throws ImageUploadException + */ + public function generateImagesByTypes(string $imagePath, array $imageTypes): bool + { + $resized = true; + + try { + foreach ($imageTypes as $imageType) { + $resized &= $this->resize($imagePath, $imageType); + } + } catch (PrestaShopException $e) { + throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); + } + + if (!$resized) { + throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); + } + + return $resized; + } + + /** + * Resizes the image depending from its type + * + * @param string $filePath + * @param string $destinationDir + * @param array $imageType + * + * @return bool + */ + private function resize(string $filePath, array $imageType): bool + { + $fileExtension = pathinfo($filePath, PATHINFO_EXTENSION); + + if (!is_file($filePath)) { + throw new ImageUploadException(sprintf('File "%s" does not exist', $filePath)); + } + + //@todo: hardcoded extension as it was in legacy code. Changing it would be a huge BC break. + //@todo: in future we should consider using extension by mimeType + $destinationExtension = 'jpg'; + $width = $imageType['width']; + $height = $imageType['height']; + + if (Configuration::get('PS_HIGHT_DPI')) { + $destinationExtension = '2x.' . $destinationExtension; + $width *= 2; + $height *= 2; + } + + return ImageManager::resize( + $filePath, + sprintf('%s-%s.%s', rtrim($filePath, $fileExtension), stripslashes($imageType['name']), $destinationExtension), + (int) $width, + (int) $height, + $destinationExtension + ); + } +} diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php new file mode 100644 index 0000000000000..02d51fea585c7 --- /dev/null +++ b/src/Adapter/Image/ImageValidator.php @@ -0,0 +1,115 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Image; + +use ImageManager; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; +use Tools; + +class ImageValidator +{ + /** + * @var string + */ + private $filePath; + + /** + * @param string $filePath + */ + public function __construct(string $filePath) + { + $this->assertIsImage($filePath); + $this->filePath = $filePath; + } + + /** + * @param string $filePath + * + * @throws ImageUploadException + * @throws UploadedImageConstraintException + */ + public function assertImageIsAllowedForUpload(): void + { + $maxFileSize = Tools::getMaxUploadSize(); + + if ($maxFileSize > 0 && filesize($this->filePath) > $maxFileSize) { + throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $image->getSize()), UploadedImageConstraintException::EXCEEDED_SIZE); + } + + if (!ImageManager::checkImageMemoryLimit($this->filePath)) { + throw new MemoryLimitException('Cannot upload image due to memory restrictions'); + } + } + + /** + * @param array $supportedExtensions + * + * @throws ImageUploadException + */ + public function assertImageTypeIsSupported(array $supportedExtensions): void + { + $mime = ImageManager::getMimeType($this->filePath); + + //@todo: add other exceptions? + if (!$mime) { + throw new ImageUploadException( + sprintf('Cannot recognize image type. Supported types are: %s', implode(',', $supportedExtensions)) + ); + } + + $extension = str_replace('image/', '', $mime); + + if (!in_array($extension, $supportedExtensions)) { + throw new ImageUploadException(sprintf( + 'Unsupported image type "%s". Supported types are: %s', + $extension, + implode(',', $supportedExtensions) + )); + } + } + + /** + * @param string $filePath + * + * @throws ImageUploadException + * @throws UploadedImageConstraintException + */ + private function assertIsImage(string $filePath): void + { + if (!is_file($filePath)) { + throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); + } + + if (!ImageManager::isRealImage($filePath, mime_content_type($filePath))) { + throw new UploadedImageConstraintException(sprintf('Image format "%s", not recognized, allowed formats are: .gif, .jpg, .png', $image->getClientOriginalExtension()), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); + } + } +} diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index b9594a27a9526..94afa13d58051 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -31,7 +31,10 @@ use ErrorException; use Hook; use Image; +use ImageType; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; +use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; +use PrestaShop\PrestaShop\Adapter\Image\ImageValidator; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; @@ -55,19 +58,27 @@ class ProductImageUploader extends AbstractImageUploader */ private $contextShopId; + /** + * @var ImageGenerator + */ + private $imageGenerator; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId + * @param ImageGenerator $imageGenerator */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, - int $contextShopId + int $contextShopId, + ImageGenerator $imageGenerator ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; + $this->imageGenerator = $imageGenerator; } /** @@ -75,11 +86,14 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { - $this->assertImageIsAllowedForUpload($filePath); + $imageValidator = new ImageValidator($filePath); + $imageValidator->assertImageIsAllowedForUpload(); + $imageValidator->assertImageTypeIsSupported(['jpeg', 'gif', 'png', 'jpg']); $this->productImagePathFactory->createDestinationDirectory($image); - $this->uploadFromTemp($filePath, $this->productImagePathFactory->getBasePath($image, true)); - $this->generateDifferentSizeImages($this->productImagePathFactory->getBasePath($image, false), 'products'); + $destinationPath = $this->productImagePathFactory->getBasePath($image); + $this->uploadFromTemp($filePath, $destinationPath); + $this->imageGenerator->generateImagesByTypes($destinationPath, ImageType::getImagesTypes('products')); Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); $this->deleteCachedImages($image); diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index 94eb58086878b..ad6956f44f4e4 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -47,7 +47,7 @@ public function __construct( $this->isLegacyImageMode = $isLegacyImageMode; } - public function getBasePath(Image $image, bool $withExtension): string + public function getBasePath(Image $image): string { if ($this->isLegacyImageMode) { $path = $image->id_product . '-' . $image->id; @@ -55,12 +55,7 @@ public function getBasePath(Image $image, bool $withExtension): string $path = $image->getImgPath(); } - //@todo: it seems that jpg is hardcoded. AdminProductsController:2836 - if ($withExtension) { - $path .= sprintf('.%s', $image->image_format); - } - - return _PS_PROD_IMG_DIR_ . $path; + return sprintf('%s%s.%s', _PS_PROD_IMG_DIR_, $path, $image->image_format); } public function createDestinationDirectory(Image $image): void @@ -75,7 +70,6 @@ public function createDestinationDirectory(Image $image): void )); } - //@todo: make static? public function getCachedCover(int $productId, int $shopId): string { return sprintf('%sproduct_mini_%s_%s.jpg', _PS_TMP_IMG_DIR_, $productId, $shopId); diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index eefd5c1c67c77..c903fcd833853 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -31,9 +31,13 @@ services: prestashop.adapter.image.uploader.supplier_image_uploader: class: 'PrestaShop\PrestaShop\Adapter\Image\Uploader\SupplierImageUploader' + prestashop.adapter.image.image_generator: + class: 'PrestaShop\PrestaShop\Adapter\Image\ImageGenerator' + prestashop.adapter.image.uploader.product_image_uploader: class: 'PrestaShop\PrestaShop\Adapter\Image\Uploader\ProductImageUploader' arguments: - '@prestashop.core.configuration.upload_size_configuration' - '@prestashop.adapter.product.product_image_path_factory' - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" + - "@prestashop.adapter.image.image_generator" From bc56d5cd748bd116d5526189c20acfcf9de869a3 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 09:16:53 +0200 Subject: [PATCH 24/87] remove redundant assert. Remove filePath from validator constructor, allow injecting it --- src/Adapter/Image/ImageValidator.php | 53 +++---------------- .../Image/Uploader/ProductImageUploader.php | 17 ++++-- .../config/services/adapter/image.yml | 4 ++ 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 02d51fea585c7..492e1f036b50b 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -29,6 +29,7 @@ namespace PrestaShop\PrestaShop\Adapter\Image; use ImageManager; +use ImageManagerCore; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; @@ -36,80 +37,40 @@ class ImageValidator { - /** - * @var string - */ - private $filePath; - - /** - * @param string $filePath - */ - public function __construct(string $filePath) - { - $this->assertIsImage($filePath); - $this->filePath = $filePath; - } - /** * @param string $filePath * * @throws ImageUploadException * @throws UploadedImageConstraintException */ - public function assertImageIsAllowedForUpload(): void + public function assertFileFitsInSystemLimits(string $filePath): void { $maxFileSize = Tools::getMaxUploadSize(); - if ($maxFileSize > 0 && filesize($this->filePath) > $maxFileSize) { + if ($maxFileSize > 0 && filesize($filePath) > $maxFileSize) { throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $image->getSize()), UploadedImageConstraintException::EXCEEDED_SIZE); } - if (!ImageManager::checkImageMemoryLimit($this->filePath)) { + if (!ImageManager::checkImageMemoryLimit($filePath)) { throw new MemoryLimitException('Cannot upload image due to memory restrictions'); } } - /** - * @param array $supportedExtensions - * - * @throws ImageUploadException - */ - public function assertImageTypeIsSupported(array $supportedExtensions): void - { - $mime = ImageManager::getMimeType($this->filePath); - - //@todo: add other exceptions? - if (!$mime) { - throw new ImageUploadException( - sprintf('Cannot recognize image type. Supported types are: %s', implode(',', $supportedExtensions)) - ); - } - - $extension = str_replace('image/', '', $mime); - - if (!in_array($extension, $supportedExtensions)) { - throw new ImageUploadException(sprintf( - 'Unsupported image type "%s". Supported types are: %s', - $extension, - implode(',', $supportedExtensions) - )); - } - } - /** * @param string $filePath * * @throws ImageUploadException * @throws UploadedImageConstraintException */ - private function assertIsImage(string $filePath): void + public function assertIsValidImage(string $filePath): void { if (!is_file($filePath)) { throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); } + $mime = mime_content_type($filePath); if (!ImageManager::isRealImage($filePath, mime_content_type($filePath))) { - throw new UploadedImageConstraintException(sprintf('Image format "%s", not recognized, allowed formats are: .gif, .jpg, .png', $image->getClientOriginalExtension()), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); + throw new UploadedImageConstraintException(sprintf('Image mime type "%s" is not allowed, allowed Types are: %s', $mime, ImageManagerCore::MIME_TYPE_SUPPORTED), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 94afa13d58051..4019290291a14 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -63,22 +63,30 @@ class ProductImageUploader extends AbstractImageUploader */ private $imageGenerator; + /** + * @var ImageValidator + */ + private $imageValidator; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId * @param ImageGenerator $imageGenerator + * @param ImageValidator $imageValidator */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, int $contextShopId, - ImageGenerator $imageGenerator + ImageGenerator $imageGenerator, + ImageValidator $imageValidator ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; $this->imageGenerator = $imageGenerator; + $this->imageValidator = $imageValidator; } /** @@ -86,11 +94,10 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { - $imageValidator = new ImageValidator($filePath); - $imageValidator->assertImageIsAllowedForUpload(); - $imageValidator->assertImageTypeIsSupported(['jpeg', 'gif', 'png', 'jpg']); - $this->productImagePathFactory->createDestinationDirectory($image); + $this->imageValidator->assertFileFitsInSystemLimits($filePath); + $this->imageValidator->assertIsValidImage($filePath); + $this->productImagePathFactory->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); $this->uploadFromTemp($filePath, $destinationPath); $this->imageGenerator->generateImagesByTypes($destinationPath, ImageType::getImagesTypes('products')); diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index c903fcd833853..7657b7778135b 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -34,6 +34,9 @@ services: prestashop.adapter.image.image_generator: class: 'PrestaShop\PrestaShop\Adapter\Image\ImageGenerator' + prestashop.adapter.image.image_validator: + class: 'PrestaShop\PrestaShop\Adapter\Image\ImageValidator' + prestashop.adapter.image.uploader.product_image_uploader: class: 'PrestaShop\PrestaShop\Adapter\Image\Uploader\ProductImageUploader' arguments: @@ -41,3 +44,4 @@ services: - '@prestashop.adapter.product.product_image_path_factory' - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" - "@prestashop.adapter.image.image_generator" + - "@prestashop.adapter.image.image_validator" From ecda9b10036c8869a4437b45216d69fe7b3b5aa2 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 09:26:39 +0200 Subject: [PATCH 25/87] rename method. Fix wrong properties in sprintf --- src/Adapter/Image/ImageValidator.php | 19 +++++++++++++------ .../Image/Uploader/ProductImageUploader.php | 5 +++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 492e1f036b50b..9b1ee28def584 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -43,12 +43,13 @@ class ImageValidator * @throws ImageUploadException * @throws UploadedImageConstraintException */ - public function assertFileFitsInSystemLimits(string $filePath): void + public function assertFileUploadLimits(string $filePath): void { $maxFileSize = Tools::getMaxUploadSize(); + $size = filesize($filePath); - if ($maxFileSize > 0 && filesize($filePath) > $maxFileSize) { - throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $image->getSize()), UploadedImageConstraintException::EXCEEDED_SIZE); + if ($maxFileSize > 0 && $size > $maxFileSize) { + throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $size), UploadedImageConstraintException::EXCEEDED_SIZE); } if (!ImageManager::checkImageMemoryLimit($filePath)) { @@ -59,18 +60,24 @@ public function assertFileFitsInSystemLimits(string $filePath): void /** * @param string $filePath * + * @param array $allowedMimeTypes + * * @throws ImageUploadException * @throws UploadedImageConstraintException */ - public function assertIsValidImage(string $filePath): void + public function assertIsValidImageType(string $filePath, array $allowedMimeTypes = null): void { + if (!$allowedMimeTypes) { + $allowedMimeTypes = ImageManagerCore::MIME_TYPE_SUPPORTED; + } + if (!is_file($filePath)) { throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); } $mime = mime_content_type($filePath); - if (!ImageManager::isRealImage($filePath, mime_content_type($filePath))) { - throw new UploadedImageConstraintException(sprintf('Image mime type "%s" is not allowed, allowed Types are: %s', $mime, ImageManagerCore::MIME_TYPE_SUPPORTED), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); + if (!ImageManager::isRealImage($filePath, mime_content_type($filePath), $allowedMimeTypes)) { + throw new UploadedImageConstraintException(sprintf('Image type "%s" is not allowed, allowed Types are: %s', $mime, implode(',', $allowedMimeTypes)), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } } diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 4019290291a14..345ced3418334 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -31,6 +31,7 @@ use ErrorException; use Hook; use Image; +use ImageManagerCore; use ImageType; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; @@ -94,8 +95,8 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { - $this->imageValidator->assertFileFitsInSystemLimits($filePath); - $this->imageValidator->assertIsValidImage($filePath); + $this->imageValidator->assertFileUploadLimits($filePath); + $this->imageValidator->assertIsValidImageType($filePath); $this->productImagePathFactory->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); From b40bf37ea2964ea3b01b97f22a0f3e668734cf94 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 09:33:47 +0200 Subject: [PATCH 26/87] use validator in handler instead of uploader --- src/Adapter/Image/ImageValidator.php | 1 - .../Image/Uploader/ProductImageUploader.php | 15 +------ .../CommandHandler/AddProductImageHandler.php | 14 ++++++- .../Command/AddProductImageCommand.php | 41 ++----------------- .../config/services/adapter/image.yml | 1 - .../config/services/adapter/product.yml | 1 + 6 files changed, 19 insertions(+), 54 deletions(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 9b1ee28def584..48a41a395d1f7 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -59,7 +59,6 @@ public function assertFileUploadLimits(string $filePath): void /** * @param string $filePath - * * @param array $allowedMimeTypes * * @throws ImageUploadException diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 345ced3418334..42c84aefb6088 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -31,11 +31,9 @@ use ErrorException; use Hook; use Image; -use ImageManagerCore; use ImageType; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; -use PrestaShop\PrestaShop\Adapter\Image\ImageValidator; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; @@ -64,30 +62,22 @@ class ProductImageUploader extends AbstractImageUploader */ private $imageGenerator; - /** - * @var ImageValidator - */ - private $imageValidator; - /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId * @param ImageGenerator $imageGenerator - * @param ImageValidator $imageValidator */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, int $contextShopId, - ImageGenerator $imageGenerator, - ImageValidator $imageValidator + ImageGenerator $imageGenerator ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; $this->imageGenerator = $imageGenerator; - $this->imageValidator = $imageValidator; } /** @@ -95,9 +85,6 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { - $this->imageValidator->assertFileUploadLimits($filePath); - $this->imageValidator->assertIsValidImageType($filePath); - $this->productImagePathFactory->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); $this->uploadFromTemp($filePath, $destinationPath); diff --git a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php index c60065a653981..2236f95c7e131 100644 --- a/src/Adapter/Product/CommandHandler/AddProductImageHandler.php +++ b/src/Adapter/Product/CommandHandler/AddProductImageHandler.php @@ -28,6 +28,7 @@ namespace PrestaShop\PrestaShop\Adapter\Product\CommandHandler; +use PrestaShop\PrestaShop\Adapter\Image\ImageValidator; use PrestaShop\PrestaShop\Adapter\Image\Uploader\ProductImageUploader; use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; @@ -54,19 +55,27 @@ final class AddProductImageHandler implements AddProductImageHandlerInterface */ private $contextShopIds; + /** + * @var ImageValidator + */ + private $imageValidator; + /** * @param ProductImageUploader $productImageUploader * @param ProductImageRepository $productImageRepository + * @param ImageValidator $imageValidator * @param array $contextShopIds */ public function __construct( ProductImageUploader $productImageUploader, ProductImageRepository $productImageRepository, + ImageValidator $imageValidator, array $contextShopIds ) { $this->productImageUploader = $productImageUploader; $this->productImageRepository = $productImageRepository; $this->contextShopIds = $contextShopIds; + $this->imageValidator = $imageValidator; } /** @@ -74,8 +83,11 @@ public function __construct( */ public function handle(AddProductImageCommand $command): ImageId { + $this->imageValidator->assertFileUploadLimits($command->getFilePath()); + $this->imageValidator->assertIsValidImageType($command->getFilePath()); + $image = $this->productImageRepository->create($command->getProductId(), $this->contextShopIds); - $this->productImageUploader->upload($image, $command->getPathName()); + $this->productImageUploader->upload($image, $command->getFilePath()); return new ImageId((int) $image->id); } diff --git a/src/Core/Domain/Product/Command/AddProductImageCommand.php b/src/Core/Domain/Product/Command/AddProductImageCommand.php index b185259c7fe5d..b196206a458b3 100644 --- a/src/Core/Domain/Product/Command/AddProductImageCommand.php +++ b/src/Core/Domain/Product/Command/AddProductImageCommand.php @@ -43,27 +43,18 @@ class AddProductImageCommand /** * @var string */ - private $originalName; - - /** - * @var string - */ - private $pathName; + private $filePath; /** * @param int $productId - * @param string $originalName * @param string $pathName */ public function __construct( int $productId, - string $originalName, string $pathName ) { $this->productId = new ProductId($productId); - //@todo: assert file constraints or leave it to uploader? - $this->originalName = $originalName; - $this->pathName = $pathName; + $this->filePath = $pathName; } /** @@ -77,32 +68,8 @@ public function getProductId(): ProductId /** * @return string */ - public function getOriginalName(): string - { - return $this->originalName; - } - - /** - * @return string - */ - public function getPathName(): string - { - return $this->pathName; - } - - /** - * @return int - */ - public function getFileSize(): int - { - return $this->fileSize; - } - - /** - * @return string - */ - public function getMimeType(): string + public function getFilePath(): string { - return $this->mimeType; + return $this->filePath; } } diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index 7657b7778135b..0920f7d988f9d 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -44,4 +44,3 @@ services: - '@prestashop.adapter.product.product_image_path_factory' - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" - "@prestashop.adapter.image.image_generator" - - "@prestashop.adapter.image.image_validator" diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 1a748980ac179..42d47a2f840ab 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -415,6 +415,7 @@ services: arguments: - '@prestashop.adapter.image.uploader.product_image_uploader' - '@prestashop.adapter.product.repository.product_image_repository' + - "@prestashop.adapter.image.image_validator" - "@=service('prestashop.adapter.shop.context').getContextListShopID()" tags: - name: tactician.handler From 3e08894816621deaa7fc33cf159e35a6f327d45b Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 10:48:50 +0200 Subject: [PATCH 27/87] use hookDispatcher & pass tmpDir as constructor arg --- .../Image/Uploader/ProductImageUploader.php | 18 +++++++++++++++--- .../Product/ProductImagePathFactory.php | 14 +++++++++++--- .../config/services/adapter/image.yml | 1 + .../config/services/adapter/product.yml | 1 + 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 42c84aefb6088..952ca06b2fe21 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -29,13 +29,13 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; use ErrorException; -use Hook; use Image; use ImageType; use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; +use PrestaShop\PrestaShop\Core\Hook\HookDispatcherInterface; /** * Uploads product image to filesystem @@ -62,22 +62,30 @@ class ProductImageUploader extends AbstractImageUploader */ private $imageGenerator; + /** + * @var HookDispatcherInterface + */ + private $hookDispatcher; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId * @param ImageGenerator $imageGenerator + * @param HookDispatcherInterface $hookDispatcher */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, int $contextShopId, - ImageGenerator $imageGenerator + ImageGenerator $imageGenerator, + HookDispatcherInterface $hookDispatcher ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; $this->imageGenerator = $imageGenerator; + $this->hookDispatcher = $hookDispatcher; } /** @@ -90,7 +98,11 @@ public function upload(Image $image, string $filePath): void $this->uploadFromTemp($filePath, $destinationPath); $this->imageGenerator->generateImagesByTypes($destinationPath, ImageType::getImagesTypes('products')); - Hook::exec('actionWatermark', ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product]); + $this->hookDispatcher->dispatchWithParameters( + 'actionWatermark', + ['id_image' => (int) $image->id, 'id_product' => (int) $image->id_product] + ); + $this->deleteCachedImages($image); } diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index ad6956f44f4e4..88a8852198401 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -38,13 +38,21 @@ class ProductImagePathFactory */ private $isLegacyImageMode; + /** + * @var string + */ + private $temporaryImgDir; + /** * @param bool $isLegacyImageMode + * @param string $temporaryImgDir */ public function __construct( - bool $isLegacyImageMode + bool $isLegacyImageMode, + string $temporaryImgDir ) { $this->isLegacyImageMode = $isLegacyImageMode; + $this->temporaryImgDir = $temporaryImgDir; } public function getBasePath(Image $image): string @@ -72,11 +80,11 @@ public function createDestinationDirectory(Image $image): void public function getCachedCover(int $productId, int $shopId): string { - return sprintf('%sproduct_mini_%s_%s.jpg', _PS_TMP_IMG_DIR_, $productId, $shopId); + return sprintf('%sproduct_mini_%s_%s.jpg', $this->temporaryImgDir, $productId, $shopId); } public function getCachedThumbnail(int $productId): string { - return sprintf('%sproduct_%s.jpg', _PS_TMP_IMG_DIR_, $productId); + return sprintf('%sproduct_%s.jpg', $this->temporaryImgDir, $productId); } } diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index 0920f7d988f9d..5a4e6b53d0fea 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -44,3 +44,4 @@ services: - '@prestashop.adapter.product.product_image_path_factory' - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" - "@prestashop.adapter.image.image_generator" + - "@prestashop.core.hook.dispatcher" diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 42d47a2f840ab..8d2b96bd312c8 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -428,3 +428,4 @@ services: class: PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory arguments: - "@=service('prestashop.adapter.legacy.configuration').getBoolean('PS_LEGACY_IMAGES')" + - !php/const _PS_TMP_IMG_DIR_ From 9b40292bec13ccaa397be90f1f651e287528368c Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 11:02:43 +0200 Subject: [PATCH 28/87] revert unnecessary out of scope changes in Abstract class --- .../Image/Uploader/AbstractImageUploader.php | 66 +++++-------------- .../Uploader/ManufacturerImageUploader.php | 56 +++++++++++++++- 2 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index b3c37818bcb88..99bad5c576aeb 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -45,37 +45,22 @@ abstract class AbstractImageUploader { /** - * @deprecated - * @see assertImageIsAllowedForUpload() - * * @param UploadedFile $image * * @throws UploadedImageConstraintException */ protected function checkImageIsAllowedForUpload(UploadedFile $image) { - $this->assertImageIsAllowedForUpload($image->getPathname()); - } - - /** - * @param string $filePath - * - * @throws ImageUploadException - * @throws UploadedImageConstraintException - */ - protected function assertImageIsAllowedForUpload(string $filePath): void - { - if (!is_file($filePath)) { - throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); - } - $maxFileSize = Tools::getMaxUploadSize(); - if ($maxFileSize > 0 && filesize($filePath) > $maxFileSize) { + if ($maxFileSize > 0 && $image->getSize() > $maxFileSize) { throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $image->getSize()), UploadedImageConstraintException::EXCEEDED_SIZE); } - if (!ImageManager::isRealImage($filePath, mime_content_type($filePath))) { + if (!ImageManager::isRealImage($image->getPathname(), $image->getClientMimeType()) + || !ImageManager::isCorrectImageFileExt($image->getClientOriginalName()) + || preg_match('/\%00/', $image->getClientOriginalName()) // prevent null byte injection + ) { throw new UploadedImageConstraintException(sprintf('Image format "%s", not recognized, allowed formats are: .gif, .jpg, .png', $image->getClientOriginalExtension()), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } @@ -125,25 +110,7 @@ protected function uploadFromTemp($temporaryImageName, $destination) /** * @deprecated use AbstractImageUploader::generateDifferentSizeImages() instead */ - protected function generateDifferentSize($id, $imageDir, $belongsTo, string $extension = 'jpg') - { - return $this->generateDifferentSizeImages( - $imageDir . $id, - $belongsTo, - $extension - ); - } - - /** - * @param string $path - * @param string $belongsTo to whom the image belongs (for example 'suppliers' or 'categories') - * @param string $extension - * - * @return bool - * - * @throws ImageOptimizationException - */ - protected function generateDifferentSizeImages(string $path, string $belongsTo, string $extension = 'jpg'): bool + protected function generateDifferentSize($id, $imageDir, $belongsTo) { $resized = true; @@ -151,12 +118,11 @@ protected function generateDifferentSizeImages(string $path, string $belongsTo, $imageTypes = ImageType::getImagesTypes($belongsTo); foreach ($imageTypes as $imageType) { - $resized &= $this->resize($path, $imageType, $extension); + $resized &= $this->resize($id, $imageDir, $imageType); } } catch (PrestaShopException $e) { throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); } - if (!$resized) { throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); } @@ -165,31 +131,31 @@ protected function generateDifferentSizeImages(string $path, string $belongsTo, } /** - * Resizes the image depending from its type + * Resizes the image depending on its type * - * @param string $path + * @param int $id + * @param string $imageDir * @param array $imageType - * @param string $extension * * @return bool */ - private function resize(string $path, array $imageType, string $extension) + private function resize($id, $imageDir, array $imageType) { + $ext = '.jpg'; $width = $imageType['width']; $height = $imageType['height']; if (Configuration::get('PS_HIGHT_DPI')) { - $extension = '2x.' . $extension; + $ext = '2x.jpg'; $width *= 2; $height *= 2; } return ImageManager::resize( - $path . '.' . $extension, - $path . '-' . stripslashes($imageType['name']) . '.' . $extension, + $imageDir . $id . '.jpg', + $imageDir . $id . '-' . stripslashes($imageType['name']) . $ext, (int) $width, - (int) $height, - $extension + (int) $height ); } } diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index 9fa6b6e65ae54..f4411ea342a1a 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -26,10 +26,12 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; +use Configuration; use ImageManager; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; +use PrestaShopException; use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -63,6 +65,58 @@ public function upload($manufacturerId, UploadedFile $image) throw new ImageOptimizationException('An error occurred while uploading the image. Check your directory permissions.'); } - $this->generateDifferentSizeImages(_PS_MANU_IMG_DIR_ . $manufacturerId, 'manufacturers'); + $this->generateDifferentSizeImages($manufacturerId); + } + + /** + * @param int $manufacturerId + * + * @return bool + */ + private function generateDifferentSizeImages($manufacturerId) + { + $resized = true; + $generateHighDpiImages = (bool) Configuration::get('PS_HIGHT_DPI'); + + try { + /* Generate images with different size */ + if (count($_FILES) && + file_exists(_PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg') + ) { + $imageTypes = ImageType::getImagesTypes('manufacturers'); + + foreach ($imageTypes as $imageType) { + $resized &= ImageManager::resize( + _PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg', + _PS_MANU_IMG_DIR_ . $manufacturerId . '-' . stripslashes($imageType['name']) . '.jpg', + (int) $imageType['width'], + (int) $imageType['height'] + ); + + if ($generateHighDpiImages) { + $resized &= ImageManager::resize( + _PS_MANU_IMG_DIR_ . $manufacturerId . '.jpg', + _PS_MANU_IMG_DIR_ . $manufacturerId . '-' . stripslashes($imageType['name']) . '2x.jpg', + (int) $imageType['width'] * 2, + (int) $imageType['height'] * 2 + ); + } + } + + $currentLogo = _PS_TMP_IMG_DIR_ . 'manufacturer_mini_' . $manufacturerId . '.jpg'; + + if ($resized && file_exists($currentLogo)) { + unlink($currentLogo); + } + } + } catch (PrestaShopException $e) { + throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); + } + + if (!$resized) { + throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); + } + + return $resized; } } From b37ec208cd80232588df038d5c157b62e24598e8 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 11:34:11 +0200 Subject: [PATCH 29/87] wrap repository method in try-catch --- .../Repository/ProductImageRepository.php | 28 ++++++++++----- .../CannotAddProductImageException.php | 36 +++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 src/Core/Domain/Product/Image/Exception/CannotAddProductImageException.php diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php index a35207e84bd18..38e1ba2848cb0 100644 --- a/src/Adapter/Product/Repository/ProductImageRepository.php +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -30,10 +30,13 @@ use Image; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\CannotAddProductImageException; +use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageException; use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageNotFoundException; use PrestaShop\PrestaShop\Core\Domain\Product\Image\ValueObject\ImageId; use PrestaShop\PrestaShop\Core\Domain\Product\ValueObject\ProductId; use PrestaShop\PrestaShop\Core\Exception\CoreException; +use PrestaShopException; /** * Provides access to product Image data source @@ -56,15 +59,24 @@ public function create(ProductId $productId, array $shopIds): Image $image->id_product = $productIdValue; $image->position = Image::getHighestPosition($productIdValue) + 1; - if (!Image::getCover($productIdValue)) { - $image->cover = 1; - } else { - $image->cover = 0; - } + $image->cover = !Image::getCover($productIdValue); + + $this->addObjectModel($image, CannotAddProductImageException::class); - //@todo: wrap in try catch - $image->add(); - $image->associateTo($shopIds); + try { + if (!$image->associateTo($shopIds)) { + throw new ProductImageException(sprintf( + 'Failed to associate product image #%d with shops', + $image->id + )); + } + } catch (PrestaShopException $e) { + throw new CoreException( + sprintf('Error occurred when trying to associate image #%d with shops', $image->id), + 0, + $e + ); + } return $image; } diff --git a/src/Core/Domain/Product/Image/Exception/CannotAddProductImageException.php b/src/Core/Domain/Product/Image/Exception/CannotAddProductImageException.php new file mode 100644 index 0000000000000..9aba819cd9888 --- /dev/null +++ b/src/Core/Domain/Product/Image/Exception/CannotAddProductImageException.php @@ -0,0 +1,36 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception; + +/** + * Thrown when adding product image fails + */ +class CannotAddProductImageException extends ProductImageException +{ +} From d2051407cf671549b95d4b5a58903bc37662b9df Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 11:34:27 +0200 Subject: [PATCH 30/87] apply cs fixer --- src/Adapter/Image/Uploader/ManufacturerImageUploader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index f4411ea342a1a..9825ee9c2bb40 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -31,8 +31,8 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; -use PrestaShopException; use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; +use PrestaShopException; use Symfony\Component\HttpFoundation\File\UploadedFile; /** From 0b14ca858e9e39bbeda568fcd2f18f0add76d33e Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 12:16:39 +0200 Subject: [PATCH 31/87] move exception to Core & add parent exception class --- .../Image/Uploader/ProductImageUploader.php | 2 +- .../Exception/CannotUnlinkImageException.php | 9 +++-- src/Core/Image/Exception/ImageException.php | 38 +++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) rename src/{Adapter => Core}/Image/Exception/CannotUnlinkImageException.php (86%) create mode 100644 src/Core/Image/Exception/ImageException.php diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 952ca06b2fe21..3a3f06bbe2908 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -31,11 +31,11 @@ use ErrorException; use Image; use ImageType; -use PrestaShop\PrestaShop\Adapter\Image\Exception\CannotUnlinkImageException; use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; use PrestaShop\PrestaShop\Core\Hook\HookDispatcherInterface; +use PrestaShop\PrestaShop\Core\Image\Exception\CannotUnlinkImageException; /** * Uploads product image to filesystem diff --git a/src/Adapter/Image/Exception/CannotUnlinkImageException.php b/src/Core/Image/Exception/CannotUnlinkImageException.php similarity index 86% rename from src/Adapter/Image/Exception/CannotUnlinkImageException.php rename to src/Core/Image/Exception/CannotUnlinkImageException.php index 6c4f28bfc2ceb..18ac017d1fcc6 100644 --- a/src/Adapter/Image/Exception/CannotUnlinkImageException.php +++ b/src/Core/Image/Exception/CannotUnlinkImageException.php @@ -26,10 +26,11 @@ declare(strict_types=1); -namespace PrestaShop\PrestaShop\Adapter\Image\Exception; +namespace PrestaShop\PrestaShop\Core\Image\Exception; -use PrestaShop\PrestaShop\Core\Exception\CoreException; - -class CannotUnlinkImageException extends CoreException +/** + * Thrown when image file unlink fails + */ +class CannotUnlinkImageException extends ImageException { } diff --git a/src/Core/Image/Exception/ImageException.php b/src/Core/Image/Exception/ImageException.php new file mode 100644 index 0000000000000..6116fff3af621 --- /dev/null +++ b/src/Core/Image/Exception/ImageException.php @@ -0,0 +1,38 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Image\Exception; + +use PrestaShop\PrestaShop\Core\Exception\CoreException; + +/** + * Base exception for image infrastructure related errors + */ +class ImageException extends CoreException +{ +} From 272fc819b28aaded1138d7cdf4c113c3f4aa9b78 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 26 Oct 2020 12:55:52 +0200 Subject: [PATCH 32/87] remove extra argument from command in test --- .../Context/Domain/Product/ProductImageFeatureContext.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 108a38e8f9523..2d1ceaa98ec1a 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -46,7 +46,6 @@ public function uploadImage(string $productReference, string $fileName) $this->getCommandBus()->handle(new AddProductImageCommand( $this->getSharedStorage()->get($productReference), - $fileName, $pathName )); } From 44991dde9227f1fa05fb6889a79559600e944df1 Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Mon, 26 Oct 2020 16:34:31 +0100 Subject: [PATCH 33/87] Fix install process in test environment for create-test-db action --- .gitignore | 5 +++++ config/defines.inc.php | 1 + src/Adapter/Language/LanguageImageManager.php | 11 ++++++++--- src/PrestaShopBundle/Install/Install.php | 2 +- tests/Resources/img/l/.gitkeep | 0 tests/Resources/img/p/.gitkeep | 0 6 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 tests/Resources/img/l/.gitkeep create mode 100644 tests/Resources/img/p/.gitkeep diff --git a/.gitignore b/.gitignore index 883b88b3cdf81..c76a171208811 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,11 @@ tests-legacy/resources/modules/ps_categorytree tests-legacy/resources/modules/ps_facetedsearch tests-legacy/resources/modules/ps_sharebuttons +tests/Resources/img/l/* +!tests/Resources/img/l/.gitkeep +tests/Resources/img/p/* +!tests/Resources/img/p/.gitkeep + /admin-dev/autoupgrade/* !/admin-dev/autoupgrade/index.php !/admin-dev/autoupgrade/backup/index.php diff --git a/config/defines.inc.php b/config/defines.inc.php index c0aa450be1e5b..0319712f9408c 100755 --- a/config/defines.inc.php +++ b/config/defines.inc.php @@ -134,6 +134,7 @@ define('_PS_TAASC_PATH_', _PS_TOOL_DIR_.'taasc/'); define('_PS_TCPDF_PATH_', _PS_TOOL_DIR_.'tcpdf/'); +define('_PS_IMG_SOURCE_DIR_', _PS_ROOT_DIR_.'/img/'); if (!defined('_PS_IMG_DIR_')) { $dir = (defined('_PS_IN_TEST_') && _PS_IN_TEST_) ? '/tests/Resources/img/' : '/img/'; define('_PS_IMG_DIR_', _PS_ROOT_DIR_.$dir); diff --git a/src/Adapter/Language/LanguageImageManager.php b/src/Adapter/Language/LanguageImageManager.php index a8cfb69645396..7df6c1fc0943e 100644 --- a/src/Adapter/Language/LanguageImageManager.php +++ b/src/Adapter/Language/LanguageImageManager.php @@ -39,10 +39,15 @@ class LanguageImageManager */ const IMG_PATH = _PS_IMG_DIR_ . '/l/'; + /** + * Path were source images are stored + */ + const IMG_SOURCE_PATH = _PS_IMG_SOURCE_DIR_ . '/l/'; + /** * Path where flags are stored */ - const FLAGS_SOURCE = _PS_IMG_DIR_ . 'flags/%s.jpg'; + const FLAGS_SOURCE = _PS_IMG_SOURCE_DIR_ . 'flags/%s.jpg'; /** * Path where flags are copied to @@ -52,7 +57,7 @@ class LanguageImageManager /** * Default flag */ - const FALLBACK_FLAG_SOURCE = self::IMG_PATH . 'none.jpg'; + const FALLBACK_FLAG_SOURCE = self::IMG_SOURCE_PATH . 'none.jpg'; const IMAGE_DIRECTORIES = [ _PS_CAT_IMG_DIR_, @@ -115,7 +120,7 @@ public function setupDefaultImagePlaceholder(string $isoCode): void foreach (static::IMAGE_DIRECTORIES as $destinationDir) { foreach ($filesToCopy as $sourceFile => $newFile) { - @copy(static::IMG_PATH . $sourceFile, $destinationDir . $newFile); + @copy(static::IMG_SOURCE_PATH . $sourceFile, $destinationDir . $newFile); } } } diff --git a/src/PrestaShopBundle/Install/Install.php b/src/PrestaShopBundle/Install/Install.php index 43fa914eda238..fb48228d2979f 100644 --- a/src/PrestaShopBundle/Install/Install.php +++ b/src/PrestaShopBundle/Install/Install.php @@ -1125,8 +1125,8 @@ public function installFixtures($entity = null, array $data = []) } } else { $xml_loader = new XmlLoader(); - $xml_loader->setTranslator($this->translator); } + $xml_loader->setTranslator($this->translator); // Install XML data (data/xml/ folder) $xml_loader->setFixturesPath($fixtures_path); diff --git a/tests/Resources/img/l/.gitkeep b/tests/Resources/img/l/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/Resources/img/p/.gitkeep b/tests/Resources/img/p/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d From 7f70b1fee621d5dc7f77825051c82f90a79be32a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 12:37:45 +0200 Subject: [PATCH 34/87] copy images state to sys tmp and restore it after feature --- .../Utils/DatabaseCreator.php | 19 +++++++++++ .../Features/Context/CommonFeatureContext.php | 34 +++---------------- .../Scenario/Product/add_image.feature | 2 +- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php index 9a62fcc781cdb..bdc6308835f90 100644 --- a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php +++ b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php @@ -30,11 +30,17 @@ use Doctrine\DBAL\DBALException; use PrestaShopBundle\Install\DatabaseDump; use PrestaShopBundle\Install\Install; +use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Process\Process; use Tab; class DatabaseCreator { + /** + * Name for directory of test images in system tmp dir + */ + const TEST_IMG_DIR = 'ps_test_img'; + /** * Create the initialize database used for test */ @@ -74,6 +80,19 @@ public static function createTestDB() $install->installModules(); DatabaseDump::create(); + + // Backs up test images directory to allow resetting their original state later in tests + (new Filesystem())->mirror(_PS_IMG_DIR_, static::getBackupTestImgDir()); + } + + /** + * Provide test img directory path, in which initial dummy images state should be saved + * + * @return string + */ + public static function getBackupTestImgDir(): string + { + return sys_get_temp_dir() . DIRECTORY_SEPARATOR . static::TEST_IMG_DIR; } /** diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index e99c75f094e60..dc593b88b2df9 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -41,6 +41,7 @@ use Pack; use Product; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Filesystem\Filesystem; use TaxManagerFactory; class CommonFeatureContext extends AbstractPrestaShopFeatureContext @@ -119,37 +120,12 @@ public static function clearDownloads(): void } /** - * @AfterFeature @clear-img-after-feature + * @AfterFeature @reset-img-after-feature */ - public static function clearImgDir(): void + public static function resetImgDir(): void { - function unlinkFilesRecursively(string $recursiveDir = null): void - { - $startingDir = _PS_IMG_DIR_; - if ($recursiveDir) { - $startingDir = $recursiveDir . '/'; - } - - $dirs = []; - foreach (glob($startingDir . '*') as $file) { - if (is_dir($file)) { - $dirs[] = $file; - - continue; - } - - if (is_file($file)) { - unlink($file); - } - } - - foreach ($dirs as $dir) { - unlinkFilesRecursively($dir); - rmdir($dir); - } - } - - unlinkFilesRecursively(); + $fs = new Filesystem(); + $fs->mirror(DatabaseCreator::getBackupTestImgDir(), _PS_IMG_DIR_); } /** diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 121d29b4d1605..446f19efe6190 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -1,7 +1,7 @@ # ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s product --tags add-image @reset-database-before-feature @clear-cache-after-feature -@clear-img-after-feature +@reset-img-after-feature @add-image Feature: Add product image from Back Office (BO) As an employee I need to be able to add new product image From 5fd160ce9c0ecd91280551639d70dedcc11e5a1f Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 13:31:44 +0200 Subject: [PATCH 35/87] implement DummyFileUploader and ResourceResetter --- .../Utils/DatabaseCreator.php | 21 +---- .../Features/Context/CommonFeatureContext.php | 5 +- .../Domain/AttachmentFeatureContext.php | 10 +-- .../Product/ProductImageFeatureContext.php | 18 +---- tests/Resources/DummyFileUploader.php | 79 +++++++++++++++++++ tests/Resources/ResourceResetter.php | 68 ++++++++++++++++ 6 files changed, 157 insertions(+), 44 deletions(-) create mode 100644 tests/Resources/DummyFileUploader.php create mode 100644 tests/Resources/ResourceResetter.php diff --git a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php index bdc6308835f90..f8930f59152c2 100644 --- a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php +++ b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php @@ -30,17 +30,12 @@ use Doctrine\DBAL\DBALException; use PrestaShopBundle\Install\DatabaseDump; use PrestaShopBundle\Install\Install; -use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Process\Process; +use Tests\Resources\ResourceResetter; use Tab; class DatabaseCreator { - /** - * Name for directory of test images in system tmp dir - */ - const TEST_IMG_DIR = 'ps_test_img'; - /** * Create the initialize database used for test */ @@ -80,19 +75,7 @@ public static function createTestDB() $install->installModules(); DatabaseDump::create(); - - // Backs up test images directory to allow resetting their original state later in tests - (new Filesystem())->mirror(_PS_IMG_DIR_, static::getBackupTestImgDir()); - } - - /** - * Provide test img directory path, in which initial dummy images state should be saved - * - * @return string - */ - public static function getBackupTestImgDir(): string - { - return sys_get_temp_dir() . DIRECTORY_SEPARATOR . static::TEST_IMG_DIR; + ResourceResetter::backupImages(); } /** diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index dc593b88b2df9..b592280138aba 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -43,6 +43,8 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\Filesystem\Filesystem; use TaxManagerFactory; +use Tests\Resources\ResourcePathProvider; +use Tests\Resources\ResourceResetter; class CommonFeatureContext extends AbstractPrestaShopFeatureContext { @@ -124,8 +126,7 @@ public static function clearDownloads(): void */ public static function resetImgDir(): void { - $fs = new Filesystem(); - $fs->mirror(DatabaseCreator::getBackupTestImgDir(), _PS_IMG_DIR_); + ResourceResetter::resetImages(); } /** diff --git a/tests/Integration/Behaviour/Features/Context/Domain/AttachmentFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/AttachmentFeatureContext.php index 112eb8b67b17a..955bf9846e1e7 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/AttachmentFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/AttachmentFeatureContext.php @@ -33,6 +33,7 @@ use PHPUnit\Framework\Assert; use PrestaShopException; use RuntimeException; +use Tests\Resources\DummyFileUploader; class AttachmentFeatureContext extends AbstractDomainFeatureContext { @@ -105,15 +106,10 @@ private function getAttachment(string $reference): Attachment */ private function uploadDummyFile(string $fileName): string { - $source = _PS_ROOT_DIR_ . '/tests/Resources/dummyFile/' . $fileName; - - if (!is_file($source)) { - throw new RuntimeException('%s is not a file', $source); - } + $file = DummyFileUploader::upload($fileName); $destination = _PS_DOWNLOAD_DIR_ . $fileName; - copy($source, $destination); - chmod($destination, 0644); + copy($file, $destination); return $destination; } diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 2d1ceaa98ec1a..9b848c4484901 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -30,6 +30,7 @@ use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; use RuntimeException; +use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext { @@ -42,26 +43,11 @@ class ProductImageFeatureContext extends AbstractProductFeatureContext public function uploadImage(string $productReference, string $fileName) { //@todo: behats database contains empty ImageType. - $pathName = $this->uploadDummyFile($fileName); + $pathName = DummyFileUploader::upload($fileName); $this->getCommandBus()->handle(new AddProductImageCommand( $this->getSharedStorage()->get($productReference), $pathName )); } - - //@todo: homogenize with attachments context method - private function uploadDummyFile(string $fileName): string - { - $source = _PS_ROOT_DIR_ . '/tests/Resources/dummyFile/' . $fileName; - - if (!is_file($source)) { - throw new RuntimeException('%s is not a file', $source); - } - - $destination = tempnam(sys_get_temp_dir(), 'PS_TEST_'); - copy($source, $destination); - - return $destination; - } } diff --git a/tests/Resources/DummyFileUploader.php b/tests/Resources/DummyFileUploader.php new file mode 100644 index 0000000000000..d9fafcd5870d0 --- /dev/null +++ b/tests/Resources/DummyFileUploader.php @@ -0,0 +1,79 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace Tests\Resources; + +use RuntimeException; + +/** + * Mimics user file uploads + */ +class DummyFileUploader +{ + /** + * Prefix for temporary files that are used as a replacement for http upload + */ + const UPLOADED_TMP_FILE_PREFIX = 'ps_upload_test_'; + + /** + * Uploads dummy file to temporary dir to mimic http file upload + * + * @param string $dummyFilename + * + * @return string destination pathname + */ + public static function upload(string $dummyFilename): string + { + $source = static::getDummyFilesPath() . $dummyFilename; + + if (!is_file($source)) { + throw new RuntimeException('file "%s" not found', $source); + } + + $destination = static::createTempFilename(); + copy($source, $destination); + + return $destination; + } + + /** + * @return string + */ + public static function getDummyFilesPath(): string + { + return _PS_ROOT_DIR_ . '/tests/Resources/dummyFile/'; + } + + /** + * @return string + */ + private static function createTempFilename(): string + { + return tempnam(sys_get_temp_dir(), static::UPLOADED_TMP_FILE_PREFIX); + } +} diff --git a/tests/Resources/ResourceResetter.php b/tests/Resources/ResourceResetter.php new file mode 100644 index 0000000000000..7fcbd202685fc --- /dev/null +++ b/tests/Resources/ResourceResetter.php @@ -0,0 +1,68 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace Tests\Resources; + +use Symfony\Component\Filesystem\Filesystem; + +/** + * @todo: better name? + */ +class ResourceResetter +{ + /** + * Name for directory of test images in system tmp dir + */ + const BACKUP_TEST_IMG_DIR = 'ps_backup_test_img'; + + /** + * Backs up test images directory to allow resetting their original state later in tests + */ + public static function backupImages(): void + { + (new Filesystem())->mirror(_PS_IMG_DIR_, static::getBackupTestImgDir()); + } + + /** + * Resets test images directory to initial state + */ + public static function resetImages(): void + { + (new Filesystem())->mirror(static::getBackupTestImgDir(), _PS_IMG_DIR_); + } + + /** + * Provide test img directory path, in which initial dummy images state should be saved + * + * @return string + */ + public static function getBackupTestImgDir(): string + { + return sys_get_temp_dir() . DIRECTORY_SEPARATOR . static::BACKUP_TEST_IMG_DIR; + } +} From e2bfcc434bddfcac652fbf3d42a798df7b246897 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 13:45:49 +0200 Subject: [PATCH 36/87] add some todos --- .../Features/Context/CommonFeatureContext.php | 2 -- .../Domain/Product/ProductImageFeatureContext.php | 13 +++++++++++-- .../Features/Scenario/Product/add_image.feature | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index b592280138aba..e20d5c235afe2 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -41,9 +41,7 @@ use Pack; use Product; use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\Filesystem\Filesystem; use TaxManagerFactory; -use Tests\Resources\ResourcePathProvider; use Tests\Resources\ResourceResetter; class CommonFeatureContext extends AbstractPrestaShopFeatureContext diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 9b848c4484901..b36695a947ee1 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -29,7 +29,6 @@ namespace Tests\Integration\Behaviour\Features\Context\Domain\Product; use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; -use RuntimeException; use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext @@ -40,7 +39,7 @@ class ProductImageFeatureContext extends AbstractProductFeatureContext * @param string $productReference * @param string $fileName */ - public function uploadImage(string $productReference, string $fileName) + public function uploadImage(string $productReference, string $fileName): void { //@todo: behats database contains empty ImageType. $pathName = DummyFileUploader::upload($fileName); @@ -50,4 +49,14 @@ public function uploadImage(string $productReference, string $fileName) $pathName )); } + + /** + * @Then product :productReference should have no images + * + * @param string $productReference + */ + public function assertProductHasNoImages(string $productReference): void + { + //@todo: query for product images? + } } diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 446f19efe6190..f8e21540061c8 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -11,5 +11,5 @@ Feature: Add product image from Back Office (BO) | name | en-US:bottle of beer | | is_virtual | false | And product "product1" type should be standard -#todo And product "product1" should have no images + And product "product1" should have no images When I add new product "product1" image "app_icon.png" From c9e6bd70e646b73b7e1c750874d005ef713fdf81 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 13:59:22 +0200 Subject: [PATCH 37/87] prepare GetProductImages query --- .../QueryHandler/GetProductImagesHandler.php | 46 +++++++++++++ .../Domain/Product/Query/GetProductImages.php | 55 +++++++++++++++ .../GetProductImagesHandlerInterface.php | 43 ++++++++++++ .../Product/QueryResult/ProductImage.php | 68 +++++++++++++++++++ .../config/services/adapter/product.yml | 6 ++ 5 files changed, 218 insertions(+) create mode 100644 src/Adapter/Product/QueryHandler/GetProductImagesHandler.php create mode 100644 src/Core/Domain/Product/Query/GetProductImages.php create mode 100644 src/Core/Domain/Product/QueryHandler/GetProductImagesHandlerInterface.php create mode 100644 src/Core/Domain/Product/QueryResult/ProductImage.php diff --git a/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php new file mode 100644 index 0000000000000..68c94589d66a1 --- /dev/null +++ b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php @@ -0,0 +1,46 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Adapter\Product\QueryHandler; + +use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; +use PrestaShop\PrestaShop\Core\Domain\Product\QueryHandler\GetProductImagesHandlerInterface; + +/** + * Handles @see GetProductImages query + */ +final class GetProductImagesHandler implements GetProductImagesHandlerInterface +{ + /** + * {@inheritdoc} + */ + public function handle(GetProductImages $query): array + { + // TODO: Implement handle() method. + } +} diff --git a/src/Core/Domain/Product/Query/GetProductImages.php b/src/Core/Domain/Product/Query/GetProductImages.php new file mode 100644 index 0000000000000..b7fd01419727e --- /dev/null +++ b/src/Core/Domain/Product/Query/GetProductImages.php @@ -0,0 +1,55 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\Query; + +use PrestaShop\PrestaShop\Core\Domain\Product\ValueObject\ProductId; + +class GetProductImages +{ + /** + * @var ProductId + */ + private $productId; + + /** + * @param ProductId $productId + */ + public function __construct(ProductId $productId) + { + $this->productId = $productId; + } + + /** + * @return ProductId + */ + public function getProductId(): ProductId + { + return $this->productId; + } +} diff --git a/src/Core/Domain/Product/QueryHandler/GetProductImagesHandlerInterface.php b/src/Core/Domain/Product/QueryHandler/GetProductImagesHandlerInterface.php new file mode 100644 index 0000000000000..ec4592464742e --- /dev/null +++ b/src/Core/Domain/Product/QueryHandler/GetProductImagesHandlerInterface.php @@ -0,0 +1,43 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +namespace PrestaShop\PrestaShop\Core\Domain\Product\QueryHandler; + +use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; +use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; + +/** + * Handles @see GetProductImages query + */ +interface GetProductImagesHandlerInterface +{ + /** + * @param GetProductImages $query + * + * @return ProductImage[] + */ + public function handle(GetProductImages $query): array; +} diff --git a/src/Core/Domain/Product/QueryResult/ProductImage.php b/src/Core/Domain/Product/QueryResult/ProductImage.php new file mode 100644 index 0000000000000..587f5e50fb0a3 --- /dev/null +++ b/src/Core/Domain/Product/QueryResult/ProductImage.php @@ -0,0 +1,68 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Domain\Product\QueryResult; + +class ProductImage +{ + /** + * @var int + */ + private $imageId; + + /** + * @var string + */ + private $path; + + /** + * @param int $imageId + * @param string $path + */ + public function __construct(int $imageId, string $path) + { + $this->imageId = $imageId; + $this->path = $path; + } + + /** + * @return int + */ + public function getImageId(): int + { + return $this->imageId; + } + + /** + * @return string + */ + public function getPath(): string + { + return $this->path; + } +} diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 8d2b96bd312c8..267460403c7b9 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -429,3 +429,9 @@ services: arguments: - "@=service('prestashop.adapter.legacy.configuration').getBoolean('PS_LEGACY_IMAGES')" - !php/const _PS_TMP_IMG_DIR_ + + prestashop.adapter.product.query_handler.get_product_images_handler: + class: PrestaShop\PrestaShop\Adapter\Product\QueryHandler\GetProductImagesHandler + tags: + - name: tactician.handler + command: PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages From e74697d2b4c87d5109b56200d4ae9e02351719f8 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 16:49:24 +0200 Subject: [PATCH 38/87] implement getProductImagesHandler --- .../QueryHandler/GetProductImagesHandler.php | 43 +++++++++++++- .../Repository/ProductImageRepository.php | 57 ++++++++++++++++++- .../Domain/Product/Query/GetProductImages.php | 9 +-- .../Product/QueryResult/ProductImage.php | 38 ++++++++++++- .../config/services/adapter/product.yml | 5 ++ 5 files changed, 144 insertions(+), 8 deletions(-) diff --git a/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php index 68c94589d66a1..a6fe0575f6613 100644 --- a/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php +++ b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php @@ -28,19 +28,60 @@ namespace PrestaShop\PrestaShop\Adapter\Product\QueryHandler; +use Image; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; use PrestaShop\PrestaShop\Core\Domain\Product\QueryHandler\GetProductImagesHandlerInterface; +use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; /** * Handles @see GetProductImages query */ final class GetProductImagesHandler implements GetProductImagesHandlerInterface { + /** + * @var ProductImageRepository + */ + private $productImageRepository; + + /** + * @param ProductImageRepository $productImageRepository + */ + public function __construct( + ProductImageRepository $productImageRepository + ) { + $this->productImageRepository = $productImageRepository; + } + /** * {@inheritdoc} */ public function handle(GetProductImages $query): array { - // TODO: Implement handle() method. + //@todo: optimize. add pagination options to query? + $images = $this->productImageRepository->getImages($query->getProductId()); + + return $this->formatImages($images); + } + + /** + * @param Image[] $images + * + * @return ProductImage[] + */ + private function formatImages(array $images): array + { + $productImages = []; + + foreach ($images as $image) { + $productImages[] = new ProductImage( + (int) $image->id, + (bool) $image->cover, + $image->legend, + $image->getImgPath() + ); + } + + return $productImages; } } diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php index 38e1ba2848cb0..215552968f4f6 100644 --- a/src/Adapter/Product/Repository/ProductImageRepository.php +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -28,6 +28,7 @@ namespace PrestaShop\PrestaShop\Adapter\Product\Repository; +use Doctrine\DBAL\Connection; use Image; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\CannotAddProductImageException; @@ -43,6 +44,28 @@ */ class ProductImageRepository extends AbstractObjectModelRepository { + /** + * @var Connection + */ + private $connection; + + /** + * @var string + */ + private $dbPrefix; + + /** + * @param Connection $connection + * @param string $dbPrefix + */ + public function __construct( + Connection $connection, + string $dbPrefix + ) { + $this->connection = $connection; + $this->dbPrefix = $dbPrefix; + } + /** * @param ProductId $productId * @param int[] $shopIds @@ -58,7 +81,6 @@ public function create(ProductId $productId, array $shopIds): Image $image = new \Image(); $image->id_product = $productIdValue; $image->position = Image::getHighestPosition($productIdValue) + 1; - $image->cover = !Image::getCover($productIdValue); $this->addObjectModel($image, CannotAddProductImageException::class); @@ -81,6 +103,39 @@ public function create(ProductId $productId, array $shopIds): Image return $image; } + /** + * @param ProductId $productId + * + * @return Image[] + * + * @throws CoreException + */ + public function getImages(ProductId $productId): array + { + $qb = $this->connection->createQueryBuilder(); + + //@todo: multishop not handled + $results = $qb->select('id_image') + ->from($this->dbPrefix . 'image', 'i') + ->where('i.id_product = :productId') + ->setParameter('productId', $productId->getValue()) + ->execute() + ->fetchAll() + ; + + if (!$results) { + return []; + } + + $images = []; + foreach ($results as $result) { + $imageId = new ImageId((int) $result['id_image']); + $images[] = $this->get($imageId); + } + + return $images; + } + /** * @param ImageId $imageId * diff --git a/src/Core/Domain/Product/Query/GetProductImages.php b/src/Core/Domain/Product/Query/GetProductImages.php index b7fd01419727e..134c43ccf7ac6 100644 --- a/src/Core/Domain/Product/Query/GetProductImages.php +++ b/src/Core/Domain/Product/Query/GetProductImages.php @@ -38,11 +38,12 @@ class GetProductImages private $productId; /** - * @param ProductId $productId + * @param int $productId */ - public function __construct(ProductId $productId) - { - $this->productId = $productId; + public function __construct( + int $productId + ) { + $this->productId = new ProductId($productId); } /** diff --git a/src/Core/Domain/Product/QueryResult/ProductImage.php b/src/Core/Domain/Product/QueryResult/ProductImage.php index 587f5e50fb0a3..4a066eae5cb95 100644 --- a/src/Core/Domain/Product/QueryResult/ProductImage.php +++ b/src/Core/Domain/Product/QueryResult/ProductImage.php @@ -35,6 +35,16 @@ class ProductImage */ private $imageId; + /** + * @var bool + */ + private $cover; + + /** + * @var array + */ + private $localizedLegends; + /** * @var string */ @@ -42,11 +52,19 @@ class ProductImage /** * @param int $imageId + * @param bool $cover + * @param array $localizedLegends * @param string $path */ - public function __construct(int $imageId, string $path) - { + public function __construct( + int $imageId, + bool $cover, + array $localizedLegends, + string $path + ) { $this->imageId = $imageId; + $this->cover = $cover; + $this->localizedLegends = $localizedLegends; $this->path = $path; } @@ -58,6 +76,22 @@ public function getImageId(): int return $this->imageId; } + /** + * @return bool + */ + public function isCover(): bool + { + return $this->cover; + } + + /** + * @return array + */ + public function getLocalizedLegends(): array + { + return $this->localizedLegends; + } + /** * @return string */ diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 267460403c7b9..23f8ba2225496 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -423,6 +423,9 @@ services: prestashop.adapter.product.repository.product_image_repository: class: PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository + arguments: + - '@doctrine.dbal.default_connection' + - '%database_prefix%' prestashop.adapter.product.product_image_path_factory: class: PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory @@ -432,6 +435,8 @@ services: prestashop.adapter.product.query_handler.get_product_images_handler: class: PrestaShop\PrestaShop\Adapter\Product\QueryHandler\GetProductImagesHandler + arguments: + - '@prestashop.adapter.product.repository.product_image_repository' tags: - name: tactician.handler command: PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages From ca7deef1e2311bcea8f3b6ef486f94702418f21a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 17:08:25 +0200 Subject: [PATCH 39/87] as ert noImages & add todo --- .../Product/ProductImageFeatureContext.php | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index b36695a947ee1..1b27560315dd5 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -28,7 +28,11 @@ namespace Tests\Integration\Behaviour\Features\Context\Domain\Product; +use Behat\Gherkin\Node\TableNode; +use PHPUnit\Framework\Assert; use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; +use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; +use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext @@ -57,6 +61,33 @@ public function uploadImage(string $productReference, string $fileName): void */ public function assertProductHasNoImages(string $productReference): void { - //@todo: query for product images? + Assert::assertEmpty( + $this->getProductImages($productReference), + sprintf('No images expected for product "%s"', $productReference) + ); + } + + /** + * @Then product :productReference should have following images: + * + * @param string $productReference + * @param TableNode $tableNode + */ + public function assertProductImages(string $productReference, TableNode $tableNode): void + { + $images = $this->getProductImages($productReference); + //@todo: how to assert images if uploaded ones doesn't have original name? + } + + /** + * @param string $productReference + * + * @return ProductImage[] + */ + private function getProductImages(string $productReference): array + { + return $this->getQueryBus()->handle(new GetProductImages( + $this->getSharedStorage()->get($productReference) + )); } } From d396ec5dbe3d5748b8ce722f054c9c62fb4e9ceb Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 18:03:38 +0200 Subject: [PATCH 40/87] assert product image & its path --- .../QueryHandler/GetProductImagesHandler.php | 3 +- .../Product/QueryResult/ProductImage.php | 19 ++++++++ .../Product/ProductImageFeatureContext.php | 45 +++++++++++++++++-- .../Scenario/Product/add_image.feature | 5 ++- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php index a6fe0575f6613..60b76646a9e53 100644 --- a/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php +++ b/src/Adapter/Product/QueryHandler/GetProductImagesHandler.php @@ -77,8 +77,9 @@ private function formatImages(array $images): array $productImages[] = new ProductImage( (int) $image->id, (bool) $image->cover, + (int) $image->position, $image->legend, - $image->getImgPath() + sprintf('%s.%s', $image->getImgPath(), $image->image_format) ); } diff --git a/src/Core/Domain/Product/QueryResult/ProductImage.php b/src/Core/Domain/Product/QueryResult/ProductImage.php index 4a066eae5cb95..3a0e9f2745075 100644 --- a/src/Core/Domain/Product/QueryResult/ProductImage.php +++ b/src/Core/Domain/Product/QueryResult/ProductImage.php @@ -28,6 +28,9 @@ namespace PrestaShop\PrestaShop\Core\Domain\Product\QueryResult; +/** + * Transfers product image data + */ class ProductImage { /** @@ -40,6 +43,11 @@ class ProductImage */ private $cover; + /** + * @var int + */ + private $position; + /** * @var array */ @@ -53,17 +61,20 @@ class ProductImage /** * @param int $imageId * @param bool $cover + * @param int $position * @param array $localizedLegends * @param string $path */ public function __construct( int $imageId, bool $cover, + int $position, array $localizedLegends, string $path ) { $this->imageId = $imageId; $this->cover = $cover; + $this->position = $position; $this->localizedLegends = $localizedLegends; $this->path = $path; } @@ -92,6 +103,14 @@ public function getLocalizedLegends(): array return $this->localizedLegends; } + /** + * @return int + */ + public function getPosition(): int + { + return $this->position; + } + /** * @return string */ diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 1b27560315dd5..cbf40402d5b64 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -33,25 +33,30 @@ use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; +use RuntimeException; +use Tests\Integration\Behaviour\Features\Context\Util\PrimitiveUtils; use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext { /** - * @When I add new product :productReference image :fileName + * @When I add new product :productReference image :imageReference named :fileName * * @param string $productReference + * @param string $imageReference * @param string $fileName */ - public function uploadImage(string $productReference, string $fileName): void + public function uploadImage(string $productReference, string $imageReference, string $fileName): void { //@todo: behats database contains empty ImageType. $pathName = DummyFileUploader::upload($fileName); - $this->getCommandBus()->handle(new AddProductImageCommand( + $imageId = $this->getCommandBus()->handle(new AddProductImageCommand( $this->getSharedStorage()->get($productReference), $pathName )); + + $this->getSharedStorage()->set($imageReference, $imageId->getValue()); } /** @@ -76,7 +81,39 @@ public function assertProductHasNoImages(string $productReference): void public function assertProductImages(string $productReference, TableNode $tableNode): void { $images = $this->getProductImages($productReference); - //@todo: how to assert images if uploaded ones doesn't have original name? + $dataRows = $tableNode->getColumnsHash(); + + Assert::assertEquals( + count($images), + count($dataRows), + 'Expected and actual images count does not match' + ); + + foreach ($dataRows as $key => $dataRow) { + $actualImage = $images[$key]; + + Assert::assertEquals( + PrimitiveUtils::castStringBooleanIntoBoolean($dataRow['is cover']), + $actualImage->isCover(), + 'Unexpected cover image' + ); + Assert::assertEquals( + $this->parseLocalizedArray($dataRow['legend']), + $actualImage->getLocalizedLegends(), + 'Unexpected image legend' + ); + Assert::assertEquals( + PrimitiveUtils::castStringIntegerIntoInteger($dataRow['position']), + $actualImage->getPosition(), + 'Unexpected image position' + ); + + $path = _PS_PROD_IMG_DIR_ . $actualImage->getPath(); + + if (!file_exists($path)) { + throw new RuntimeException(sprintf('File "%s" does not exist', $path)); + } + } } /** diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index f8e21540061c8..9efef5a72f8bb 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -12,4 +12,7 @@ Feature: Add product image from Back Office (BO) | is_virtual | false | And product "product1" type should be standard And product "product1" should have no images - When I add new product "product1" image "app_icon.png" + When I add new product "product1" image "image1" named "app_icon.png" + Then product "product1" should have following images: + | image reference | is cover | legend | position | + | image1 | true | en-US: | 1 | From 8f0d754d9ec67603289f653a7f0e780152d217a9 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 27 Oct 2020 18:04:55 +0200 Subject: [PATCH 41/87] add todo --- .../Behaviour/Features/Scenario/Product/add_image.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 9efef5a72f8bb..9fb6884b66010 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -16,3 +16,4 @@ Feature: Add product image from Back Office (BO) Then product "product1" should have following images: | image reference | is cover | legend | position | | image1 | true | en-US: | 1 | +#todo: assert image types (which is not present in dummy database), assert combination & pack images From 6de479e0fed99f50901c0d32d8f3c1cb4595deee Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 09:58:21 +0200 Subject: [PATCH 42/87] add test case --- .../Behaviour/Features/Scenario/Product/add_image.feature | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 9fb6884b66010..ef253bf311814 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -16,4 +16,10 @@ Feature: Add product image from Back Office (BO) Then product "product1" should have following images: | image reference | is cover | legend | position | | image1 | true | en-US: | 1 | + When I add new product "product1" image "image2" named "logo.jpg" + Then product "product1" should have following images: + | image reference | is cover | legend | position | + | image1 | true | en-US: | 1 | + | image2 | false | en-US: | 2 | + #todo: assert image types (which is not present in dummy database), assert combination & pack images From bdecd8159d8d735ab5afd9df71ff9b6ae348757e Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 10:33:50 +0200 Subject: [PATCH 43/87] add todo & fix some docblocks --- src/Adapter/Image/ImageGenerator.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index b8eb3b8c62721..bd866f17e64be 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -54,6 +54,8 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo $resized &= $this->resize($imagePath, $imageType); } } catch (PrestaShopException $e) { + //@todo: this exception extends generic Exception and is in Uploader ns. + //@todo: could it extend new Core/Image/ImageException? this would introduce BC break. (make the same with other Uploader/Exception's ?) throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); } @@ -65,10 +67,9 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo } /** - * Resizes the image depending from its type + * Resizes the image depending on its type * * @param string $filePath - * @param string $destinationDir * @param array $imageType * * @return bool From 7019dbe534969852fc4b24c933c7fc91257f9805 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 15:11:33 +0200 Subject: [PATCH 44/87] move ImageOptimizationException. Extend ImageException --- src/Adapter/Image/ImageGenerator.php | 4 +--- src/Adapter/Image/Uploader/AbstractImageUploader.php | 2 +- src/Adapter/Image/Uploader/CategoryCoverImageUploader.php | 2 +- src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php | 2 +- src/Adapter/Image/Uploader/ManufacturerImageUploader.php | 2 +- .../{Uploader => }/Exception/ImageOptimizationException.php | 4 ++-- src/Core/Image/Uploader/Exception/ImageUploadException.php | 4 ++-- src/Core/Image/Uploader/Exception/MemoryLimitException.php | 4 ++-- .../Uploader/Exception/UploadedImageConstraintException.php | 4 ++-- .../Controller/Admin/Sell/Catalog/ManufacturerController.php | 2 +- .../Controller/Admin/Sell/Catalog/SupplierController.php | 2 +- 11 files changed, 15 insertions(+), 17 deletions(-) rename src/Core/Image/{Uploader => }/Exception/ImageOptimizationException.php (91%) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index bd866f17e64be..275ebefe54222 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -30,7 +30,7 @@ use Configuration; use ImageManager; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShopException; @@ -54,8 +54,6 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo $resized &= $this->resize($imagePath, $imageType); } } catch (PrestaShopException $e) { - //@todo: this exception extends generic Exception and is in Uploader ns. - //@todo: could it extend new Core/Image/ImageException? this would introduce BC break. (make the same with other Uploader/Exception's ?) throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); } diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index 99bad5c576aeb..0037f53b48d48 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -29,7 +29,7 @@ use Configuration; use ImageManager; use ImageType; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; diff --git a/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php b/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php index 205db6ad5a49e..24c5e546f1df4 100644 --- a/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php +++ b/src/Adapter/Image/Uploader/CategoryCoverImageUploader.php @@ -29,7 +29,7 @@ use Category; use ImageManager; use ImageType; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; diff --git a/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php b/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php index a45c0aef14e0f..7e152c45655c1 100644 --- a/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php +++ b/src/Adapter/Image/Uploader/CategoryThumbnailImageUploader.php @@ -28,7 +28,7 @@ use ImageManager; use ImageType; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index 9825ee9c2bb40..61b2a7cd9a1af 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -28,7 +28,7 @@ use Configuration; use ImageManager; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; diff --git a/src/Core/Image/Uploader/Exception/ImageOptimizationException.php b/src/Core/Image/Exception/ImageOptimizationException.php similarity index 91% rename from src/Core/Image/Uploader/Exception/ImageOptimizationException.php rename to src/Core/Image/Exception/ImageOptimizationException.php index 8c42ab19b4834..e6e749fd9f1d0 100644 --- a/src/Core/Image/Uploader/Exception/ImageOptimizationException.php +++ b/src/Core/Image/Exception/ImageOptimizationException.php @@ -24,11 +24,11 @@ * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) */ -namespace PrestaShop\PrestaShop\Core\Image\Uploader\Exception; +namespace PrestaShop\PrestaShop\Core\Image\Exception; /** * Class ImageOptimizationException is thrown when resizing, cutting or optimizing image fails. */ -class ImageOptimizationException extends \Exception +class ImageOptimizationException extends ImageException { } diff --git a/src/Core/Image/Uploader/Exception/ImageUploadException.php b/src/Core/Image/Uploader/Exception/ImageUploadException.php index d26e7ad15eea7..5241ca525472f 100644 --- a/src/Core/Image/Uploader/Exception/ImageUploadException.php +++ b/src/Core/Image/Uploader/Exception/ImageUploadException.php @@ -26,8 +26,8 @@ namespace PrestaShop\PrestaShop\Core\Image\Uploader\Exception; -use PrestaShop\PrestaShop\Core\Exception\CoreException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageException; -class ImageUploadException extends CoreException +class ImageUploadException extends ImageException { } diff --git a/src/Core/Image/Uploader/Exception/MemoryLimitException.php b/src/Core/Image/Uploader/Exception/MemoryLimitException.php index 95f1cf9298bec..1a9c6e6c7be8a 100644 --- a/src/Core/Image/Uploader/Exception/MemoryLimitException.php +++ b/src/Core/Image/Uploader/Exception/MemoryLimitException.php @@ -26,8 +26,8 @@ namespace PrestaShop\PrestaShop\Core\Image\Uploader\Exception; -use PrestaShop\PrestaShop\Core\Exception\CoreException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageException; -class MemoryLimitException extends CoreException +class MemoryLimitException extends ImageException { } diff --git a/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php b/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php index c8adb3311a3c3..143ab4b8b1caf 100644 --- a/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php +++ b/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php @@ -26,9 +26,9 @@ namespace PrestaShop\PrestaShop\Core\Image\Uploader\Exception; -use PrestaShop\PrestaShop\Core\Exception\CoreException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageException; -class UploadedImageConstraintException extends CoreException +class UploadedImageConstraintException extends ImageException { const EXCEEDED_SIZE = 1; const UNRECOGNIZED_FORMAT = 2; diff --git a/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/ManufacturerController.php b/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/ManufacturerController.php index d40e75a23aa3d..f8b81e403c3b3 100644 --- a/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/ManufacturerController.php +++ b/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/ManufacturerController.php @@ -54,7 +54,7 @@ use PrestaShop\PrestaShop\Core\Form\IdentifiableObject\Handler\FormHandlerInterface; use PrestaShop\PrestaShop\Core\Grid\Definition\Factory\ManufacturerAddressGridDefinitionFactory; use PrestaShop\PrestaShop\Core\Grid\Definition\Factory\ManufacturerGridDefinitionFactory; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; diff --git a/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/SupplierController.php b/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/SupplierController.php index 48ae7f132b9a7..e4e6364a04e05 100644 --- a/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/SupplierController.php +++ b/src/PrestaShopBundle/Controller/Admin/Sell/Catalog/SupplierController.php @@ -44,7 +44,7 @@ use PrestaShop\PrestaShop\Core\Domain\Supplier\QueryResult\ViewableSupplier; use PrestaShop\PrestaShop\Core\Form\IdentifiableObject\Builder\FormBuilderInterface; use PrestaShop\PrestaShop\Core\Form\IdentifiableObject\Handler\FormHandlerInterface; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageOptimizationException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; From 49e2abc90fc7a85827cf2f5ef1cd10cb324792f5 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 15:38:57 +0200 Subject: [PATCH 45/87] Use ImageType instead of array --- src/Adapter/Image/ImageGenerator.php | 18 ++++++---- .../Repository/ProductImageRepository.php | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index 275ebefe54222..8c453ad0f1015 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -30,15 +30,19 @@ use Configuration; use ImageManager; +use ImageType; use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShopException; +/** + * Responsible for resizing images based on provided types + */ class ImageGenerator { /** * @param string $imagePath - * @param array $imageTypes + * @param ImageType[] $imageTypes * * @return bool * @@ -68,11 +72,11 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo * Resizes the image depending on its type * * @param string $filePath - * @param array $imageType + * @param ImageType $imageType * * @return bool */ - private function resize(string $filePath, array $imageType): bool + private function resize(string $filePath, ImageType $imageType): bool { $fileExtension = pathinfo($filePath, PATHINFO_EXTENSION); @@ -83,8 +87,8 @@ private function resize(string $filePath, array $imageType): bool //@todo: hardcoded extension as it was in legacy code. Changing it would be a huge BC break. //@todo: in future we should consider using extension by mimeType $destinationExtension = 'jpg'; - $width = $imageType['width']; - $height = $imageType['height']; + $width = $imageType->width; + $height = $imageType->height; if (Configuration::get('PS_HIGHT_DPI')) { $destinationExtension = '2x.' . $destinationExtension; @@ -95,8 +99,8 @@ private function resize(string $filePath, array $imageType): bool return ImageManager::resize( $filePath, sprintf('%s-%s.%s', rtrim($filePath, $fileExtension), stripslashes($imageType['name']), $destinationExtension), - (int) $width, - (int) $height, + $width, + $height, $destinationExtension ); } diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php index 215552968f4f6..e149e88a6e31a 100644 --- a/src/Adapter/Product/Repository/ProductImageRepository.php +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -30,6 +30,7 @@ use Doctrine\DBAL\Connection; use Image; +use ImageType; use PrestaShop\PrestaShop\Adapter\AbstractObjectModelRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\CannotAddProductImageException; use PrestaShop\PrestaShop\Core\Domain\Product\Image\Exception\ProductImageException; @@ -136,6 +137,39 @@ public function getImages(ProductId $productId): array return $images; } + /** + * @return ImageType[] + */ + public function getProductImageTypes(): array + { + try { + $results = ImageType::getImagesTypes('products'); + } catch (PrestaShopException $e) { + throw new CoreException('Error occurred when trying to get product image types'); + } + + if (!$results) { + return []; + } + + $imageTypes = []; + foreach ($results as $result) { + $imageType = new ImageType(); + $imageType->id = (int) $result['id_image_type']; + $imageType->name = $result['name']; + $imageType->width = (int) $result['width']; + $imageType->height = (int) $result['height']; + $imageType->products = (bool) $result['products']; + $imageType->categories = (bool) $result['categories']; + $imageType->manufacturers = (bool) $result['manufacturers']; + $imageType->suppliers = (bool) $result['suppliers']; + $imageType->stores = (bool) $result['stores']; + $imageTypes[] = $imageType; + } + + return $imageTypes; + } + /** * @param ImageId $imageId * From 5fe10436a8bb4f925b29a830a3a213d92f0615cf Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 15:42:57 +0200 Subject: [PATCH 46/87] add docblock --- src/Adapter/Image/ImageValidator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 48a41a395d1f7..7c4324fa6a9b2 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -35,6 +35,9 @@ use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; use Tools; +/** + * Responsible for validating image before upload + */ class ImageValidator { /** From 051fd321678f11a2e820968b6887d80a7e33a399 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 15:54:42 +0200 Subject: [PATCH 47/87] make method protected to open extension ability --- src/Adapter/Image/ImageGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index 8c453ad0f1015..574e8eb1f2c0a 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -76,7 +76,7 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo * * @return bool */ - private function resize(string $filePath, ImageType $imageType): bool + protected function resize(string $filePath, ImageType $imageType): bool { $fileExtension = pathinfo($filePath, PATHINFO_EXTENSION); From 06615a9f590d202951f83f1b2c794798a0971fd1 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 16:00:17 +0200 Subject: [PATCH 48/87] add optional uploadSize param in constructor --- src/Adapter/Image/ImageValidator.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 7c4324fa6a9b2..6100084115446 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -40,6 +40,22 @@ */ class ImageValidator { + /** + * @var int + */ + private $maxUploadSize; + + /** + * @param int|null $maxUploadSize + */ + public function __construct(?int $maxUploadSize = null) + { + if (null === $maxUploadSize) { + $maxUploadSize = Tools::getMaxUploadSize(); + } + $this->maxUploadSize = $maxUploadSize; + } + /** * @param string $filePath * @@ -48,10 +64,9 @@ class ImageValidator */ public function assertFileUploadLimits(string $filePath): void { - $maxFileSize = Tools::getMaxUploadSize(); $size = filesize($filePath); - if ($maxFileSize > 0 && $size > $maxFileSize) { + if ($this->maxUploadSize > 0 && $size > $this->maxUploadSize) { throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $size), UploadedImageConstraintException::EXCEEDED_SIZE); } From ec06f8843902df30cf7841ae34d1070b834f5207 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 16:08:00 +0200 Subject: [PATCH 49/87] remove redundant position value (its handled in ObjModel) --- src/Adapter/Product/Repository/ProductImageRepository.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Adapter/Product/Repository/ProductImageRepository.php b/src/Adapter/Product/Repository/ProductImageRepository.php index e149e88a6e31a..096c6945daf77 100644 --- a/src/Adapter/Product/Repository/ProductImageRepository.php +++ b/src/Adapter/Product/Repository/ProductImageRepository.php @@ -73,15 +73,15 @@ public function __construct( * * @return Image * - * @throws \PrestaShopDatabaseException - * @throws \PrestaShopException + * @throws CoreException + * @throws ProductImageException + * @throws CannotAddProductImageException */ public function create(ProductId $productId, array $shopIds): Image { $productIdValue = $productId->getValue(); - $image = new \Image(); + $image = new Image(); $image->id_product = $productIdValue; - $image->position = Image::getHighestPosition($productIdValue) + 1; $image->cover = !Image::getCover($productIdValue); $this->addObjectModel($image, CannotAddProductImageException::class); From dc743d5ac5d170ba344b0c909bb0336769a9985a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 16:09:08 +0200 Subject: [PATCH 50/87] fix Imagetype docblock types --- classes/ImageType.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/classes/ImageType.php b/classes/ImageType.php index 1fd6a9c1d52de..8801f8c1b6464 100644 --- a/classes/ImageType.php +++ b/classes/ImageType.php @@ -43,16 +43,16 @@ class ImageTypeCore extends ObjectModel /** @var bool Apply to products */ public $products; - /** @var int Apply to categories */ + /** @var bool Apply to categories */ public $categories; - /** @var int Apply to manufacturers */ + /** @var bool Apply to manufacturers */ public $manufacturers; - /** @var int Apply to suppliers */ + /** @var bool Apply to suppliers */ public $suppliers; - /** @var int Apply to store */ + /** @var bool Apply to store */ public $stores; /** From de79229092a44c2f5482601d23474e6b2a833ca4 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 16:20:45 +0200 Subject: [PATCH 51/87] fix image validator property --- src/Adapter/Image/ImageValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 6100084115446..984ec4731f5f9 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -67,7 +67,7 @@ public function assertFileUploadLimits(string $filePath): void $size = filesize($filePath); if ($this->maxUploadSize > 0 && $size > $this->maxUploadSize) { - throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $maxFileSize, $size), UploadedImageConstraintException::EXCEEDED_SIZE); + throw new UploadedImageConstraintException(sprintf('Max file size allowed is "%s" bytes. Uploaded image size is "%s".', $this->maxUploadSize, $size), UploadedImageConstraintException::EXCEEDED_SIZE); } if (!ImageManager::checkImageMemoryLimit($filePath)) { From 97a223b5d164ca743ce52055e3afe42df7c1a4bb Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 17:38:56 +0200 Subject: [PATCH 52/87] add unit tests for ImageValidator --- src/Adapter/Image/ImageValidator.php | 5 +- .../Exception/ImageFileNotFoundException.php | 38 ++++++ tests/Resources/DummyFileUploader.php | 2 +- tests/Resources/dummyFile/test_text_file.txt | 1 + .../Unit/Adapter/Image/ImageValidatorTest.php | 118 ++++++++++++++++++ 5 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 src/Core/Image/Uploader/Exception/ImageFileNotFoundException.php create mode 100644 tests/Resources/dummyFile/test_text_file.txt create mode 100644 tests/Unit/Adapter/Image/ImageValidatorTest.php diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 984ec4731f5f9..f7961d7f1d9e5 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -30,6 +30,7 @@ use ImageManager; use ImageManagerCore; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageFileNotFoundException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; @@ -89,12 +90,12 @@ public function assertIsValidImageType(string $filePath, array $allowedMimeTypes } if (!is_file($filePath)) { - throw new ImageUploadException(sprintf('"%s" is not a file', $filePath)); + throw new ImageFileNotFoundException(sprintf('Image file "%s" not found', $filePath)); } $mime = mime_content_type($filePath); if (!ImageManager::isRealImage($filePath, mime_content_type($filePath), $allowedMimeTypes)) { - throw new UploadedImageConstraintException(sprintf('Image type "%s" is not allowed, allowed Types are: %s', $mime, implode(',', $allowedMimeTypes)), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); + throw new UploadedImageConstraintException(sprintf('Image type "%s" is not allowed, allowed types are: %s', $mime, implode(',', $allowedMimeTypes)), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } } diff --git a/src/Core/Image/Uploader/Exception/ImageFileNotFoundException.php b/src/Core/Image/Uploader/Exception/ImageFileNotFoundException.php new file mode 100644 index 0000000000000..e9f09c99c9f99 --- /dev/null +++ b/src/Core/Image/Uploader/Exception/ImageFileNotFoundException.php @@ -0,0 +1,38 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace PrestaShop\PrestaShop\Core\Image\Uploader\Exception; + +use PrestaShop\PrestaShop\Core\Image\Exception\ImageException; + +/** + * Thrown when file by provided path was not found + */ +class ImageFileNotFoundException extends ImageException +{ +} diff --git a/tests/Resources/DummyFileUploader.php b/tests/Resources/DummyFileUploader.php index d9fafcd5870d0..4d07f188ce82b 100644 --- a/tests/Resources/DummyFileUploader.php +++ b/tests/Resources/DummyFileUploader.php @@ -66,7 +66,7 @@ public static function upload(string $dummyFilename): string */ public static function getDummyFilesPath(): string { - return _PS_ROOT_DIR_ . '/tests/Resources/dummyFile/'; + return __DIR__ . '/dummyFile/'; } /** diff --git a/tests/Resources/dummyFile/test_text_file.txt b/tests/Resources/dummyFile/test_text_file.txt new file mode 100644 index 0000000000000..d2cf010d36ff3 --- /dev/null +++ b/tests/Resources/dummyFile/test_text_file.txt @@ -0,0 +1 @@ +Lorem ipsum dolor sit amet. diff --git a/tests/Unit/Adapter/Image/ImageValidatorTest.php b/tests/Unit/Adapter/Image/ImageValidatorTest.php new file mode 100644 index 0000000000000..a97315c443bf1 --- /dev/null +++ b/tests/Unit/Adapter/Image/ImageValidatorTest.php @@ -0,0 +1,118 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace Tests\Unit\Adapter\Image; + +use Generator; +use PHPUnit\Framework\TestCase; +use PrestaShop\PrestaShop\Adapter\Image\ImageValidator; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageFileNotFoundException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\UploadedImageConstraintException; +use Tests\Resources\DummyFileUploader; + +class ImageValidatorTest extends TestCase +{ + /** + * @var ImageValidator + */ + private $imageValidator; + + public function setUp() + { + require_once '../../bootstrap.php'; + $this->imageValidator = new ImageValidator(); + } + + /** + * @dataProvider getInvalidMaxUploadSizesForFile + * + * @param string $filePath + * @param int $maxUploadSize + */ + public function testItThrowsExceptionWhenFileSizeIsLargerThanMaxUploadSize(string $filePath, int $maxUploadSize) + { + $imageValidator = new ImageValidator($maxUploadSize); + $this->expectException(UploadedImageConstraintException::class); + $this->expectExceptionCode(UploadedImageConstraintException::EXCEEDED_SIZE); + + $imageValidator->assertFileUploadLimits($filePath); + } + + /** + * @dataProvider getUnsupportedImageTypes + * + * @param string $filePath + * @param array|null $allowedTypes + * + * @throws UploadedImageConstraintException + * @throws ImageUploadException + */ + public function testItThrowsExceptionWhenUnsupportedImageTypeIsProvided(string $filePath, ?array $allowedTypes): void + { + $this->expectException(UploadedImageConstraintException::class); + $this->expectExceptionCode(UploadedImageConstraintException::UNRECOGNIZED_FORMAT); + + $this->imageValidator->assertIsValidImageType($filePath, $allowedTypes); + } + + /** + * @dataProvider getInvalidPathsToAFile + * + * @param string $filePath + */ + public function testItThrowsExceptionWhenFileDoesNotExistByProvidedPath(string $filePath): void + { + $this->expectException(ImageFileNotFoundException::class); + $this->imageValidator->assertIsValidImageType($filePath); + } + + public function getInvalidMaxUploadSizesForFile(): Generator + { + $logoPath = DummyFileUploader::getDummyFilesPath() . 'logo.jpg'; + $appIconPath = DummyFileUploader::getDummyFilesPath() . 'app_icon.png'; + + yield [$logoPath, 2500]; + yield [$logoPath, 2750]; + yield [$appIconPath, 1900]; + yield [$appIconPath, 100]; + } + + public function getUnsupportedImageTypes(): Generator + { + // mime type of logo.jpg is "image/jpeg" (not image/jpg) that is why logo.jpg should not be allowed in following case + yield [DummyFileUploader::getDummyFilesPath() . 'logo.jpg', ['image/jpg', 'image/png', 'image/gif']]; + yield [DummyFileUploader::getDummyFilesPath() . 'app_icon.png', ['image/jpg', 'image/gif']]; + yield [DummyFileUploader::getDummyFilesPath() . 'test_text_file.txt', null]; + } + + public function getInvalidPathsToAFile(): Generator + { + yield ['its/definately/notafile', __DIR__]; + } +} From d517a6bc2ed61aa1b1ce4c65ced4246b42265c6a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 28 Oct 2020 18:34:44 +0200 Subject: [PATCH 53/87] add missing import in ManufacturerImageUpload --- src/Adapter/Image/Uploader/ManufacturerImageUploader.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php index 61b2a7cd9a1af..e5989586f42eb 100644 --- a/src/Adapter/Image/Uploader/ManufacturerImageUploader.php +++ b/src/Adapter/Image/Uploader/ManufacturerImageUploader.php @@ -28,6 +28,7 @@ use Configuration; use ImageManager; +use ImageType; use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; From bbe40e9f6a328730fb771e35f4b927b02fd951bf Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 29 Oct 2020 09:15:13 +0200 Subject: [PATCH 54/87] fix accessing object name --- src/Adapter/Image/ImageGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index 574e8eb1f2c0a..aafbeb3f0f65c 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -98,7 +98,7 @@ protected function resize(string $filePath, ImageType $imageType): bool return ImageManager::resize( $filePath, - sprintf('%s-%s.%s', rtrim($filePath, $fileExtension), stripslashes($imageType['name']), $destinationExtension), + sprintf('%s-%s.%s', rtrim($filePath, $fileExtension), stripslashes($imageType->name), $destinationExtension), $width, $height, $destinationExtension From 5b9a222fd6bafb261f91dbae22263692132f4c45 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 29 Oct 2020 16:02:40 +0200 Subject: [PATCH 55/87] fix path to bootstrap --- tests/Unit/Adapter/Image/ImageValidatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Adapter/Image/ImageValidatorTest.php b/tests/Unit/Adapter/Image/ImageValidatorTest.php index a97315c443bf1..7c7b97d34a6b8 100644 --- a/tests/Unit/Adapter/Image/ImageValidatorTest.php +++ b/tests/Unit/Adapter/Image/ImageValidatorTest.php @@ -45,7 +45,7 @@ class ImageValidatorTest extends TestCase public function setUp() { - require_once '../../bootstrap.php'; + require_once __DIR__ . '/../../bootstrap.php'; $this->imageValidator = new ImageValidator(); } From fb56a3cfe753fb4e04d6797fc07a57df45b5c220 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Thu, 29 Oct 2020 16:04:26 +0200 Subject: [PATCH 56/87] use shorthand if in constructor --- src/Adapter/Image/ImageValidator.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index f7961d7f1d9e5..1f26ef9a6a780 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -51,10 +51,7 @@ class ImageValidator */ public function __construct(?int $maxUploadSize = null) { - if (null === $maxUploadSize) { - $maxUploadSize = Tools::getMaxUploadSize(); - } - $this->maxUploadSize = $maxUploadSize; + $this->maxUploadSize = $maxUploadSize ?: Tools::getMaxUploadSize(); } /** From aeb971fba7e168439adf35d5ee1a25e518aa0881 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 10:48:54 +0200 Subject: [PATCH 57/87] fix ImageGenerator extension string --- src/Adapter/Image/ImageGenerator.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index aafbeb3f0f65c..c778c4f014521 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -86,19 +86,19 @@ protected function resize(string $filePath, ImageType $imageType): bool //@todo: hardcoded extension as it was in legacy code. Changing it would be a huge BC break. //@todo: in future we should consider using extension by mimeType - $destinationExtension = 'jpg'; + $destinationExtension = '.jpg'; $width = $imageType->width; $height = $imageType->height; if (Configuration::get('PS_HIGHT_DPI')) { - $destinationExtension = '2x.' . $destinationExtension; + $destinationExtension = '2x' . $destinationExtension; $width *= 2; $height *= 2; } return ImageManager::resize( $filePath, - sprintf('%s-%s.%s', rtrim($filePath, $fileExtension), stripslashes($imageType->name), $destinationExtension), + sprintf('%s-%s%s', rtrim($filePath, $fileExtension), stripslashes($imageType->name), $destinationExtension), $width, $height, $destinationExtension From e8adb3427ccf7f04ae7369ef5d78484b114d3c6c Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 10:52:25 +0200 Subject: [PATCH 58/87] use var --- src/Adapter/Image/ImageValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageValidator.php b/src/Adapter/Image/ImageValidator.php index 1f26ef9a6a780..4bb3a87a3d5a0 100644 --- a/src/Adapter/Image/ImageValidator.php +++ b/src/Adapter/Image/ImageValidator.php @@ -91,7 +91,7 @@ public function assertIsValidImageType(string $filePath, array $allowedMimeTypes } $mime = mime_content_type($filePath); - if (!ImageManager::isRealImage($filePath, mime_content_type($filePath), $allowedMimeTypes)) { + if (!ImageManager::isRealImage($filePath, $mime, $allowedMimeTypes)) { throw new UploadedImageConstraintException(sprintf('Image type "%s" is not allowed, allowed types are: %s', $mime, implode(',', $allowedMimeTypes)), UploadedImageConstraintException::UNRECOGNIZED_FORMAT); } } From 9f3c5cffa5b8017734af5fda8fb63251c15bbfd5 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 10:54:01 +0200 Subject: [PATCH 59/87] revert outdated comment --- src/Adapter/Image/Uploader/AbstractImageUploader.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index 0037f53b48d48..40b6792e6148f 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -108,7 +108,15 @@ protected function uploadFromTemp($temporaryImageName, $destination) } /** - * @deprecated use AbstractImageUploader::generateDifferentSizeImages() instead + * Generates different size images + * + * @param int $id + * @param string $imageDir + * @param string $belongsTo to whom the image belongs (for example 'suppliers' or 'categories') + * + * @return bool + * + * @throws ImageOptimizationException */ protected function generateDifferentSize($id, $imageDir, $belongsTo) { From bc6d3c34418384853b9a856008472bf0b7e77d35 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 13:17:12 +0200 Subject: [PATCH 60/87] rename cached images methods (it was reversed) --- src/Adapter/Product/ProductImagePathFactory.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index 88a8852198401..b65e2172ccec8 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -78,13 +78,13 @@ public function createDestinationDirectory(Image $image): void )); } - public function getCachedCover(int $productId, int $shopId): string + public function getCachedCover(int $productId): string { - return sprintf('%sproduct_mini_%s_%s.jpg', $this->temporaryImgDir, $productId, $shopId); + return sprintf('%sproduct_%d.jpg', $this->temporaryImgDir, $productId); } - public function getCachedThumbnail(int $productId): string + public function getCachedThumbnail(int $productId, int $shopId): string { - return sprintf('%sproduct_%s.jpg', $this->temporaryImgDir, $productId); + return sprintf('%sproduct_mini_%d_%d.jpg', $this->temporaryImgDir, $productId, $shopId); } } From 0c93e3ad791544a9253a1a66fe235f643ac4752d Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 14:03:49 +0200 Subject: [PATCH 61/87] rename method --- .../Image/Uploader/ProductImageUploader.php | 2 +- .../Product/ProductImagePathFactory.php | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 3a3f06bbe2908..5ddc550a6be6e 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -117,7 +117,7 @@ private function deleteCachedImages(Image $image): void $cachedImages = [ $this->productImagePathFactory->getCachedCover($productId, $this->contextShopId), - $this->productImagePathFactory->getCachedThumbnail($productId), + $this->productImagePathFactory->getHelperThumbnail($productId), ]; foreach ($cachedImages as $cachedImage) { diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index b65e2172ccec8..50303777cf8bb 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -55,6 +55,11 @@ public function __construct( $this->temporaryImgDir = $temporaryImgDir; } + /** + * @param Image $image + * + * @return string + */ public function getBasePath(Image $image): string { if ($this->isLegacyImageMode) { @@ -66,6 +71,11 @@ public function getBasePath(Image $image): string return sprintf('%s%s.%s', _PS_PROD_IMG_DIR_, $path, $image->image_format); } + /** + * @param Image $image + * + * @throws ImageUploadException + */ public function createDestinationDirectory(Image $image): void { if ($this->isLegacyImageMode || $image->createImgFolder()) { @@ -78,12 +88,23 @@ public function createDestinationDirectory(Image $image): void )); } + /** + * @param int $productId + * + * @return string + */ public function getCachedCover(int $productId): string { return sprintf('%sproduct_%d.jpg', $this->temporaryImgDir, $productId); } - public function getCachedThumbnail(int $productId, int $shopId): string + /** + * @param int $productId + * @param int $shopId + * + * @return string + */ + public function getHelperThumbnail(int $productId, int $shopId): string { return sprintf('%sproduct_mini_%d_%d.jpg', $this->temporaryImgDir, $productId, $shopId); } From 844479ae892121850c2a74ed4ae8dd75fe4f791b Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 14:12:15 +0200 Subject: [PATCH 62/87] move imgDir creation to uploader --- .../Image/Uploader/ProductImageUploader.php | 34 ++++++++++++++++--- .../Product/ProductImagePathFactory.php | 18 ---------- .../config/services/adapter/image.yml | 1 + 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 5ddc550a6be6e..eac3a773cd4f2 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -36,6 +36,7 @@ use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; use PrestaShop\PrestaShop\Core\Hook\HookDispatcherInterface; use PrestaShop\PrestaShop\Core\Image\Exception\CannotUnlinkImageException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; /** * Uploads product image to filesystem @@ -67,25 +68,33 @@ class ProductImageUploader extends AbstractImageUploader */ private $hookDispatcher; + /** + * @var bool + */ + private $isLegacyImageMode; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory * @param int $contextShopId * @param ImageGenerator $imageGenerator * @param HookDispatcherInterface $hookDispatcher + * @param bool $isLegacyImageMode */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, ProductImagePathFactory $productImagePathFactory, int $contextShopId, ImageGenerator $imageGenerator, - HookDispatcherInterface $hookDispatcher + HookDispatcherInterface $hookDispatcher, + bool $isLegacyImageMode ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; $this->contextShopId = $contextShopId; $this->imageGenerator = $imageGenerator; $this->hookDispatcher = $hookDispatcher; + $this->isLegacyImageMode = $isLegacyImageMode; } /** @@ -93,7 +102,7 @@ public function __construct( */ public function upload(Image $image, string $filePath): void { - $this->productImagePathFactory->createDestinationDirectory($image); + $this->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); $this->uploadFromTemp($filePath, $destinationPath); $this->imageGenerator->generateImagesByTypes($destinationPath, ImageType::getImagesTypes('products')); @@ -106,6 +115,23 @@ public function upload(Image $image, string $filePath): void $this->deleteCachedImages($image); } + /** + * @param Image $image + * + * @throws ImageUploadException + */ + private function createDestinationDirectory(Image $image): void + { + if ($this->isLegacyImageMode || $image->createImgFolder()) { + return; + } + + throw new ImageUploadException(sprintf( + 'Error occurred when trying to create directory for product #%s image', + $image->id_product + )); + } + /** * @param Image $image * @@ -116,8 +142,8 @@ private function deleteCachedImages(Image $image): void $productId = (int) $image->id_product; $cachedImages = [ - $this->productImagePathFactory->getCachedCover($productId, $this->contextShopId), - $this->productImagePathFactory->getHelperThumbnail($productId), + $this->productImagePathFactory->getHelperThumbnail($productId, $this->contextShopId), + $this->productImagePathFactory->getCachedCover($productId), ]; foreach ($cachedImages as $cachedImage) { diff --git a/src/Adapter/Product/ProductImagePathFactory.php b/src/Adapter/Product/ProductImagePathFactory.php index 50303777cf8bb..0cd3f8054ec8f 100644 --- a/src/Adapter/Product/ProductImagePathFactory.php +++ b/src/Adapter/Product/ProductImagePathFactory.php @@ -29,7 +29,6 @@ namespace PrestaShop\PrestaShop\Adapter\Product; use Image; -use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; class ProductImagePathFactory { @@ -71,23 +70,6 @@ public function getBasePath(Image $image): string return sprintf('%s%s.%s', _PS_PROD_IMG_DIR_, $path, $image->image_format); } - /** - * @param Image $image - * - * @throws ImageUploadException - */ - public function createDestinationDirectory(Image $image): void - { - if ($this->isLegacyImageMode || $image->createImgFolder()) { - return; - } - - throw new ImageUploadException(sprintf( - 'Error occurred when trying to create directory for product #%s image', - $image->id_product - )); - } - /** * @param int $productId * diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index 5a4e6b53d0fea..cf89f5490be0f 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -45,3 +45,4 @@ services: - "@=service('prestashop.adapter.legacy.context').getContext().shop.id" - "@prestashop.adapter.image.image_generator" - "@prestashop.core.hook.dispatcher" + - "@=service('prestashop.adapter.legacy.configuration').getBoolean('PS_LEGACY_IMAGES')" From 63ec5a9113f79254403efe659df7702cd04ca16f Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 14:25:03 +0200 Subject: [PATCH 63/87] add todo for future refactoring --- src/Adapter/Image/Uploader/ProductImageUploader.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index eac3a773cd4f2..9400779b0d946 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -122,12 +122,13 @@ public function upload(Image $image, string $filePath): void */ private function createDestinationDirectory(Image $image): void { + //@todo: refactor this to some new service which relies on ImagePathFactory & uses symfony Filesystem if ($this->isLegacyImageMode || $image->createImgFolder()) { return; } throw new ImageUploadException(sprintf( - 'Error occurred when trying to create directory for product #%s image', + 'Error occurred when trying to create directory for product #%d image', $image->id_product )); } From 0a0b3ec072b7704c3a95c19c0f5e90b124f3b759 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 15:35:45 +0200 Subject: [PATCH 64/87] provide original file type for image::resize --- src/Adapter/Image/ImageGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index c778c4f014521..7a67978977da3 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -101,7 +101,7 @@ protected function resize(string $filePath, ImageType $imageType): bool sprintf('%s-%s%s', rtrim($filePath, $fileExtension), stripslashes($imageType->name), $destinationExtension), $width, $height, - $destinationExtension + trim(mime_content_type($filePath), 'image/') ); } } From a4a47c31091999c7b63a5879fc76411cc2c7a804 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 3 Nov 2020 19:00:42 +0200 Subject: [PATCH 65/87] refactor resource resetter & use it for attachment dir too --- .../Utils/DatabaseCreator.php | 5 +- .../Features/Context/CommonFeatureContext.php | 12 ++-- .../Attachment/attachment_management.feature | 2 +- .../Product/update_attachments.feature | 2 +- tests/Resources/ResourceResetter.php | 65 +++++++++++++++++-- 5 files changed, 68 insertions(+), 18 deletions(-) diff --git a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php index f8930f59152c2..13c93531e0e2b 100644 --- a/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php +++ b/tests-legacy/PrestaShopBundle/Utils/DatabaseCreator.php @@ -75,7 +75,10 @@ public static function createTestDB() $install->installModules(); DatabaseDump::create(); - ResourceResetter::backupImages(); + + $resourceResetter = new ResourceResetter(); + $resourceResetter->backupImages(); + $resourceResetter->backupDownloads(); } /** diff --git a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php index e20d5c235afe2..e9ba19a1b2c81 100644 --- a/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/CommonFeatureContext.php @@ -108,15 +108,11 @@ public static function getContainer() } /** - * @AfterFeature @clear-downloads-after-feature + * @AfterFeature @reset-downloads-after-feature */ - public static function clearDownloads(): void + public static function resetDownloads(): void { - foreach (glob(_PS_DOWNLOAD_DIR_ . '*') as $file) { - if (is_file($file)) { - unlink($file); - } - } + (new ResourceResetter())->resetDownloads(); } /** @@ -124,7 +120,7 @@ public static function clearDownloads(): void */ public static function resetImgDir(): void { - ResourceResetter::resetImages(); + (new ResourceResetter())->resetImages(); } /** diff --git a/tests/Integration/Behaviour/Features/Scenario/Attachment/attachment_management.feature b/tests/Integration/Behaviour/Features/Scenario/Attachment/attachment_management.feature index 60eeefc39b7c4..c2cb1a5805be7 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Attachment/attachment_management.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Attachment/attachment_management.feature @@ -1,7 +1,7 @@ # ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s attachment @reset-database-before-feature -@clear-downloads-after-feature @clear-cache-after-feature +@reset-downloads-after-feature Feature: Manage attachment from Back Office (BO) As an employee I want to be able to add, update and delete attachments diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/update_attachments.feature b/tests/Integration/Behaviour/Features/Scenario/Product/update_attachments.feature index 917248c1dfb94..951a4fcb1f13c 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/update_attachments.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/update_attachments.feature @@ -2,7 +2,7 @@ @reset-database-before-feature @update-attachments @clear-cache-after-feature -@clear-downloads-after-feature +@reset-downloads-after-feature Feature: Update product attachments from Back Office (BO). As an employee I want to be able to assign/remove existing attachments to product and add new ones. diff --git a/tests/Resources/ResourceResetter.php b/tests/Resources/ResourceResetter.php index 7fcbd202685fc..28d5e170fdc59 100644 --- a/tests/Resources/ResourceResetter.php +++ b/tests/Resources/ResourceResetter.php @@ -31,29 +31,70 @@ use Symfony\Component\Filesystem\Filesystem; /** - * @todo: better name? + * Backups and reverts testable resources to its initial state. */ class ResourceResetter { + /** + * @var Filesystem|null + */ + private $filesystem; + + /** + * @var string|null + */ + private $backupRootDir; + + /** + * @param Filesystem|null $filesystem + * @param string|null $backupRootDir + */ + public function __construct( + ?Filesystem $filesystem = null, + ?string $backupRootDir = null + ) { + $this->filesystem = $filesystem ?: new Filesystem(); + $this->backupRootDir = $backupRootDir ?: sys_get_temp_dir(); + } + /** * Name for directory of test images in system tmp dir */ const BACKUP_TEST_IMG_DIR = 'ps_backup_test_img'; + const BACKUP_TEST_DOWNLOADS_DIR = 'ps_backup_test_download'; /** * Backs up test images directory to allow resetting their original state later in tests */ - public static function backupImages(): void + public function backupImages(): void + { + $this->filesystem->mirror(_PS_IMG_DIR_, $this->getBackupTestImgDir()); + } + + /** + * Backs up test downloads directory to allow resetting their original state later in tests + */ + public function backupDownloads(): void + { + $this->filesystem->mirror(_PS_DOWNLOAD_DIR_, $this->getBackupTestDownloadsDir()); + } + + /** + * Resets test images directory to initial state + */ + public function resetImages(): void { - (new Filesystem())->mirror(_PS_IMG_DIR_, static::getBackupTestImgDir()); + $this->filesystem->remove(_PS_IMG_DIR_); + $this->filesystem->mirror($this->getBackupTestImgDir(), _PS_IMG_DIR_); } /** * Resets test images directory to initial state */ - public static function resetImages(): void + public function resetDownloads(): void { - (new Filesystem())->mirror(static::getBackupTestImgDir(), _PS_IMG_DIR_); + $this->filesystem->remove(_PS_DOWNLOAD_DIR_); + $this->filesystem->mirror($this->getBackupTestDownloadsDir(), _PS_DOWNLOAD_DIR_); } /** @@ -61,8 +102,18 @@ public static function resetImages(): void * * @return string */ - public static function getBackupTestImgDir(): string + public function getBackupTestImgDir(): string + { + return $this->backupRootDir . DIRECTORY_SEPARATOR . static::BACKUP_TEST_IMG_DIR; + } + + /** + * Provide test downloads directory path, in which initial downloads state should be saved + * + * @return string + */ + public function getBackupTestDownloadsDir(): string { - return sys_get_temp_dir() . DIRECTORY_SEPARATOR . static::BACKUP_TEST_IMG_DIR; + return $this->backupRootDir . DIRECTORY_SEPARATOR . static::BACKUP_TEST_DOWNLOADS_DIR; } } From 4843153255b370e754d5d3747c3326869d513496 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 4 Nov 2020 13:27:52 +0200 Subject: [PATCH 66/87] use self instead of static in ResourceResetter --- tests/Resources/ResourceResetter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Resources/ResourceResetter.php b/tests/Resources/ResourceResetter.php index 28d5e170fdc59..f4322cdd2e43c 100644 --- a/tests/Resources/ResourceResetter.php +++ b/tests/Resources/ResourceResetter.php @@ -104,7 +104,7 @@ public function resetDownloads(): void */ public function getBackupTestImgDir(): string { - return $this->backupRootDir . DIRECTORY_SEPARATOR . static::BACKUP_TEST_IMG_DIR; + return $this->backupRootDir . DIRECTORY_SEPARATOR . self::BACKUP_TEST_IMG_DIR; } /** @@ -114,6 +114,6 @@ public function getBackupTestImgDir(): string */ public function getBackupTestDownloadsDir(): string { - return $this->backupRootDir . DIRECTORY_SEPARATOR . static::BACKUP_TEST_DOWNLOADS_DIR; + return $this->backupRootDir . DIRECTORY_SEPARATOR . self::BACKUP_TEST_DOWNLOADS_DIR; } } From 4d4bb2ae6fffc08131bc53cc5934f16f7281f1c6 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 6 Nov 2020 12:21:54 +0200 Subject: [PATCH 67/87] decleare public visibility to consts --- .../Exception/ProductImageConstraintException.php | 2 +- .../Exception/UploadedImageConstraintException.php | 6 +++--- tests/Resources/DummyFileUploader.php | 2 +- tests/Resources/ResourceResetter.php | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php b/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php index 090e9af706eb3..1f6faf9a63995 100644 --- a/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php +++ b/src/Core/Domain/Product/Image/Exception/ProductImageConstraintException.php @@ -36,5 +36,5 @@ class ProductImageConstraintException extends ProductImageException /** * When image id is invalid */ - const INVALID_ID = 10; + public const INVALID_ID = 10; } diff --git a/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php b/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php index 143ab4b8b1caf..bc6cffeb29955 100644 --- a/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php +++ b/src/Core/Image/Uploader/Exception/UploadedImageConstraintException.php @@ -30,7 +30,7 @@ class UploadedImageConstraintException extends ImageException { - const EXCEEDED_SIZE = 1; - const UNRECOGNIZED_FORMAT = 2; - const UNKNOWN_ERROR = 4; + public const EXCEEDED_SIZE = 1; + public const UNRECOGNIZED_FORMAT = 2; + public const UNKNOWN_ERROR = 4; } diff --git a/tests/Resources/DummyFileUploader.php b/tests/Resources/DummyFileUploader.php index 4d07f188ce82b..cc4499759f529 100644 --- a/tests/Resources/DummyFileUploader.php +++ b/tests/Resources/DummyFileUploader.php @@ -38,7 +38,7 @@ class DummyFileUploader /** * Prefix for temporary files that are used as a replacement for http upload */ - const UPLOADED_TMP_FILE_PREFIX = 'ps_upload_test_'; + public const UPLOADED_TMP_FILE_PREFIX = 'ps_upload_test_'; /** * Uploads dummy file to temporary dir to mimic http file upload diff --git a/tests/Resources/ResourceResetter.php b/tests/Resources/ResourceResetter.php index f4322cdd2e43c..5c791edc6328a 100644 --- a/tests/Resources/ResourceResetter.php +++ b/tests/Resources/ResourceResetter.php @@ -35,6 +35,12 @@ */ class ResourceResetter { + /** + * Name for directory of test images in system tmp dir + */ + public const BACKUP_TEST_IMG_DIR = 'ps_backup_test_img'; + public const BACKUP_TEST_DOWNLOADS_DIR = 'ps_backup_test_download'; + /** * @var Filesystem|null */ @@ -57,12 +63,6 @@ public function __construct( $this->backupRootDir = $backupRootDir ?: sys_get_temp_dir(); } - /** - * Name for directory of test images in system tmp dir - */ - const BACKUP_TEST_IMG_DIR = 'ps_backup_test_img'; - const BACKUP_TEST_DOWNLOADS_DIR = 'ps_backup_test_download'; - /** * Backs up test images directory to allow resetting their original state later in tests */ From fa454869a8957618ab66506de524b116e33a98b3 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 6 Nov 2020 18:24:08 +0200 Subject: [PATCH 68/87] Implement ImageUploaderInterface in recently merged new class --- src/Adapter/Image/Uploader/ProfileImageUploader.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Adapter/Image/Uploader/ProfileImageUploader.php b/src/Adapter/Image/Uploader/ProfileImageUploader.php index 159db79008d29..c5ac128f54c3d 100644 --- a/src/Adapter/Image/Uploader/ProfileImageUploader.php +++ b/src/Adapter/Image/Uploader/ProfileImageUploader.php @@ -28,13 +28,14 @@ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; +use PrestaShop\PrestaShop\Core\Image\Uploader\ImageUploaderInterface; use Profile; use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Uploads profile logo image */ -final class ProfileImageUploader extends AbstractImageUploader +final class ProfileImageUploader extends AbstractImageUploader implements ImageUploaderInterface { /** * @var string From 6e49255d8c4e034d75b522e7ce140b422aad0ff7 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 9 Nov 2020 16:55:41 +0200 Subject: [PATCH 69/87] image uploader return destination path --- .../Image/Uploader/ProductImageUploader.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index 9400779b0d946..ba378792dd654 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -36,7 +36,9 @@ use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; use PrestaShop\PrestaShop\Core\Hook\HookDispatcherInterface; use PrestaShop\PrestaShop\Core\Image\Exception\CannotUnlinkImageException; +use PrestaShop\PrestaShop\Core\Image\Exception\ImageOptimizationException; use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\ImageUploadException; +use PrestaShop\PrestaShop\Core\Image\Uploader\Exception\MemoryLimitException; /** * Uploads product image to filesystem @@ -98,9 +100,17 @@ public function __construct( } /** - * {@inheritdoc} + * @param Image $image + * @param string $filePath + * + * @return string destination path of main image + * + * @throws CannotUnlinkImageException + * @throws ImageUploadException + * @throws ImageOptimizationException + * @throws MemoryLimitException */ - public function upload(Image $image, string $filePath): void + public function upload(Image $image, string $filePath): string { $this->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); @@ -113,6 +123,8 @@ public function upload(Image $image, string $filePath): void ); $this->deleteCachedImages($image); + + return $destinationPath; } /** From 306a5434dc2ff354088d98fafdde7bd75eee9bef Mon Sep 17 00:00:00 2001 From: matks Date: Tue, 10 Nov 2020 15:57:47 +0100 Subject: [PATCH 70/87] Add tests/Resources/img/tmp folder --- tests/Resources/img/tmp/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/Resources/img/tmp/.gitkeep diff --git a/tests/Resources/img/tmp/.gitkeep b/tests/Resources/img/tmp/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d From 2339b885b33ffd0da7197a2a1d2d079beaf67f43 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 13 Nov 2020 16:16:50 +0200 Subject: [PATCH 71/87] add test dirs genders and os. Adjust gitignore --- .gitignore | 4 ++++ tests/Resources/img/genders/.gitkeep | 0 tests/Resources/img/os/.gitkeep | 0 3 files changed, 4 insertions(+) create mode 100644 tests/Resources/img/genders/.gitkeep create mode 100644 tests/Resources/img/os/.gitkeep diff --git a/.gitignore b/.gitignore index c76a171208811..628d1cd3d4900 100644 --- a/.gitignore +++ b/.gitignore @@ -107,6 +107,10 @@ tests/Resources/img/l/* !tests/Resources/img/l/.gitkeep tests/Resources/img/p/* !tests/Resources/img/p/.gitkeep +tests/Resources/img/os/* +!tests/Resources/img/os/.gitkeep +tests/Resources/img/genders/* +!tests/Resources/img/genders/.gitkeep /admin-dev/autoupgrade/* !/admin-dev/autoupgrade/index.php diff --git a/tests/Resources/img/genders/.gitkeep b/tests/Resources/img/genders/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/Resources/img/os/.gitkeep b/tests/Resources/img/os/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d From b7675b92c2a5b44041ef5c5e7e295fac5c9a8fce Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 13 Nov 2020 18:44:13 +0200 Subject: [PATCH 72/87] clear cachebefore feature --- .../Behaviour/Features/Scenario/Product/add_image.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index ef253bf311814..4d139aba4d227 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -1,6 +1,6 @@ # ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s product --tags add-image @reset-database-before-feature -@clear-cache-after-feature +@clear-cache-before-feature @reset-img-after-feature @add-image Feature: Add product image from Back Office (BO) From a0b207750f35f95f0e0039e3eb6c7d6b9060d99c Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 16 Nov 2020 11:07:24 +0200 Subject: [PATCH 73/87] add category img folder in test resources --- .gitignore | 4 ++++ tests/Resources/img/c/.gitkeep | 0 2 files changed, 4 insertions(+) create mode 100644 tests/Resources/img/c/.gitkeep diff --git a/.gitignore b/.gitignore index 628d1cd3d4900..98301f36e3c17 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,10 @@ tests/Resources/img/os/* !tests/Resources/img/os/.gitkeep tests/Resources/img/genders/* !tests/Resources/img/genders/.gitkeep +tests/Resources/img/c/* +!tests/Resources/img/c/.gitkeep +tests/Resources/img/tmp/* +tests/Resources/img/tmp/.gitkeep /admin-dev/autoupgrade/* !/admin-dev/autoupgrade/index.php diff --git a/tests/Resources/img/c/.gitkeep b/tests/Resources/img/c/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d From cb4df8a7be3009b08eb9ecbda58faaa905e8d533 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 16 Nov 2020 11:09:17 +0200 Subject: [PATCH 74/87] add todo --- .../Features/Context/Domain/CategoryFeatureContext.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/CategoryFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/CategoryFeatureContext.php index 3eea2ddc187ab..90cf129e73f33 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/CategoryFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/CategoryFeatureContext.php @@ -535,6 +535,7 @@ private function mapDataToEditableCategory( ); } + //@todo: useless test. Must retrieve it from db. return new EditableCategory( new CategoryId($categoryId), $name, @@ -585,6 +586,7 @@ private function getParentCategoryId(array $testCaseData) */ private function pretendImageUploaded(array $testCaseData, int $categoryId): string { + //@todo: refactor CategoryCoverUploader. Move uploaded file in Form handler instead of Uploader and use the uploader here in tests $categoryCoverImageName = $testCaseData['Category cover image']; $data = base64_decode(self::JPG_IMAGE_STRING); $im = imagecreatefromstring($data); From 144f1b6f912f51d7e0488c2383170b28bea05b11 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 16 Nov 2020 16:51:33 +0200 Subject: [PATCH 75/87] remove duplicated scenario from product creation --- .../Features/Scenario/Product/add_product.feature | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature index 6292d5c1f7c23..dcc7b980f4f0d 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature @@ -12,17 +12,8 @@ Feature: Add basic product from Back Office (BO) | is_virtual | false | Then product "product1" should be disabled And product "product1" type should be standard - And product "product1" localized "name" should be "en-US:bottle of beer" - And product "product1" should be assigned to default category - - Scenario: I add a product with basic information - When I add product "product1" with following information: - | name | en-US:bottle of beer | - | is_virtual | true | - Then product "product1" should be disabled - Then product "product1" should have following options information: + And product "product1" should have following options information: | condition | new | - And product "product1" type should be virtual And product "product1" localized "name" should be "en-US:bottle of beer" And product "product1" should be assigned to default category From f12dadf9befa94cb93d72617e93280ca969b03bc Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Mon, 16 Nov 2020 16:53:31 +0200 Subject: [PATCH 76/87] clear cache before and after --- .../Behaviour/Features/Scenario/Product/add_product.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature index dcc7b980f4f0d..49e63c69641ee 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_product.feature @@ -1,5 +1,6 @@ # ./vendor/bin/behat -c tests/Integration/Behaviour/behat.yml -s product --tags add @reset-database-before-feature +@clear-cache-before-feature @clear-cache-after-feature @add Feature: Add basic product from Back Office (BO) From be4f4a0d0ecbffadcba46b7a1ef43929203ca6c0 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 10:31:24 +0200 Subject: [PATCH 77/87] fix image generator error --- src/Adapter/Image/ImageGenerator.php | 2 +- .../Image/Uploader/ProductImageUploader.php | 14 +++++++++++--- .../Resources/config/services/adapter/image.yml | 1 + 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index 7a67978977da3..345e9656ea130 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -65,7 +65,7 @@ public function generateImagesByTypes(string $imagePath, array $imageTypes): boo throw new ImageOptimizationException('Unable to resize one or more of your pictures.'); } - return $resized; + return (bool) $resized; } /** diff --git a/src/Adapter/Image/Uploader/ProductImageUploader.php b/src/Adapter/Image/Uploader/ProductImageUploader.php index ba378792dd654..3265439852beb 100644 --- a/src/Adapter/Image/Uploader/ProductImageUploader.php +++ b/src/Adapter/Image/Uploader/ProductImageUploader.php @@ -30,9 +30,9 @@ use ErrorException; use Image; -use ImageType; use PrestaShop\PrestaShop\Adapter\Image\ImageGenerator; use PrestaShop\PrestaShop\Adapter\Product\ProductImagePathFactory; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Configuration\UploadSizeConfigurationInterface; use PrestaShop\PrestaShop\Core\Hook\HookDispatcherInterface; use PrestaShop\PrestaShop\Core\Image\Exception\CannotUnlinkImageException; @@ -75,6 +75,11 @@ class ProductImageUploader extends AbstractImageUploader */ private $isLegacyImageMode; + /** + * @var ProductImageRepository + */ + private $productImageRepository; + /** * @param UploadSizeConfigurationInterface $uploadSizeConfiguration * @param ProductImagePathFactory $productImagePathFactory @@ -82,6 +87,7 @@ class ProductImageUploader extends AbstractImageUploader * @param ImageGenerator $imageGenerator * @param HookDispatcherInterface $hookDispatcher * @param bool $isLegacyImageMode + * @param ProductImageRepository $productImageRepository */ public function __construct( UploadSizeConfigurationInterface $uploadSizeConfiguration, @@ -89,7 +95,8 @@ public function __construct( int $contextShopId, ImageGenerator $imageGenerator, HookDispatcherInterface $hookDispatcher, - bool $isLegacyImageMode + bool $isLegacyImageMode, + ProductImageRepository $productImageRepository ) { $this->uploadSizeConfiguration = $uploadSizeConfiguration; $this->productImagePathFactory = $productImagePathFactory; @@ -97,6 +104,7 @@ public function __construct( $this->imageGenerator = $imageGenerator; $this->hookDispatcher = $hookDispatcher; $this->isLegacyImageMode = $isLegacyImageMode; + $this->productImageRepository = $productImageRepository; } /** @@ -115,7 +123,7 @@ public function upload(Image $image, string $filePath): string $this->createDestinationDirectory($image); $destinationPath = $this->productImagePathFactory->getBasePath($image); $this->uploadFromTemp($filePath, $destinationPath); - $this->imageGenerator->generateImagesByTypes($destinationPath, ImageType::getImagesTypes('products')); + $this->imageGenerator->generateImagesByTypes($destinationPath, $this->productImageRepository->getProductImageTypes()); $this->hookDispatcher->dispatchWithParameters( 'actionWatermark', diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml index cf89f5490be0f..43302a07f0654 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/image.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/image.yml @@ -46,3 +46,4 @@ services: - "@prestashop.adapter.image.image_generator" - "@prestashop.core.hook.dispatcher" - "@=service('prestashop.adapter.legacy.configuration').getBoolean('PS_LEGACY_IMAGES')" + - '@prestashop.adapter.product.repository.product_image_repository' From 8a2c2ef6a23860d2a0d5a12b30bc6ee8ec020149 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 11:41:06 +0200 Subject: [PATCH 78/87] add StringArrayTransform --- .../Transform/StringArrayTransform.php | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php diff --git a/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php b/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php new file mode 100644 index 0000000000000..8e614b01b518a --- /dev/null +++ b/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php @@ -0,0 +1,49 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) + */ + +declare(strict_types=1); + +namespace Tests\Integration\Behaviour\Features\Transform; + +use Tests\Integration\Behaviour\Features\Context\Util\PrimitiveUtils; + +/** + * Trait containing string to array | array to string transformations + */ +trait StringArrayTransform +{ + /** + * @Transform /"\[.*?\]"$/ + * + * @param string $string + * + * @return array + */ + public function transformStringArrayToArray(string $string): array + { + return PrimitiveUtils::castStringArrayIntoArray($string); + } +} From 27c3fc162227914b0cdebfd247b7ce8953fef32c Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 12:04:17 +0200 Subject: [PATCH 79/87] wip. Add assertion for generated images --- .../Product/ProductImageFeatureContext.php | 69 +++++++++++++++++++ .../Scenario/Product/add_image.feature | 18 ++++- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index cbf40402d5b64..9cbb7fec62f81 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -30,15 +30,29 @@ use Behat\Gherkin\Node\TableNode; use PHPUnit\Framework\Assert; +use PrestaShop\PrestaShop\Adapter\Product\Repository\ProductImageRepository; use PrestaShop\PrestaShop\Core\Domain\Product\Command\AddProductImageCommand; use PrestaShop\PrestaShop\Core\Domain\Product\Query\GetProductImages; use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\PrimitiveUtils; +use Tests\Integration\Behaviour\Features\Transform\StringArrayTransform; use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext { + use StringArrayTransform; + + /** + * @var ProductImageRepository + */ + private $productImageRepository; + + public function __construct() + { + $this->productImageRepository = $this->getContainer()->get('prestashop.adapter.product.repository.product_image_repository'); + } + /** * @When I add new product :productReference image :imageReference named :fileName * @@ -59,6 +73,61 @@ public function uploadImage(string $productReference, string $imageReference, st $this->getSharedStorage()->set($imageReference, $imageId->getValue()); } + /** + * @Given following image types should be applicable to products: + */ + public function assertProductsImageTypesExists(TableNode $tableNode) + { + $dataRows = $tableNode->getColumnsHash(); + $imageTypes = $this->productImageRepository->getProductImageTypes(); + + Assert::assertEquals( + count($dataRows), + count($imageTypes), + 'Expected and actual image types count does not match' + ); + + foreach ($dataRows as $key => $expectedType) { + $actualType = $imageTypes[$key]; + Assert::assertEquals($expectedType['name'], $actualType->name, 'Unexpected image type name'); + Assert::assertEquals($expectedType['width'], $actualType->width, 'Unexpected image type width'); + Assert::assertEquals($expectedType['height'], $actualType->width, 'Unexpected image type height'); + + $this->getSharedStorage()->set($expectedType['reference'], (int) $actualType->id); + } + } + + /** + * @Given /^images "\[.*?\]" should have following types generated:$/ + * + * @Transform("\[.*?\]") + * + * @param string[] $imageReferences + * @param TableNode $tableNode + */ + public function assertProductImageTypesGenerated(array $imageReferences, TableNode $tableNode) + { + $dataRows = $tableNode->getColumnsHash(); + + foreach ($imageReferences as $imageReference) { + $imageId = $this->getSharedStorage()->get($imageReference); + foreach ($dataRows as $dataRow) { + $imgPath = $this->parseGeneratedImagePath($imageId, $dataRow['name']); + if (file_exists($imgPath)) { + //@todo: assert file dimensions + } + } + } + } + + private function parseGeneratedImagePath(int $imageId, string $imageTypeName): string + { + $directories = str_split((string) $imageId); + $path = implode('/', $directories); + + return _PS_IMG_DIR_ . $path . '/' . $imageId . '-' . $imageTypeName . '.jpg'; + } + /** * @Then product :productReference should have no images * diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 4d139aba4d227..2cd7cead17de9 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -7,7 +7,14 @@ Feature: Add product image from Back Office (BO) As an employee I need to be able to add new product image Scenario: Add new product image - Given I add product "product1" with following information: + Given following image types should be applicable to products: + | reference | name | width | height | + | cartDefault | cart_default | 125 | 125 | + | homeDefault | home_default | 250 | 250 | + | largeDefault | large_default | 800 | 800 | + | mediumDefault | medium_default | 452 | 452 | + | smallDefault | small_default | 98 | 98 | + And I add product "product1" with following information: | name | en-US:bottle of beer | | is_virtual | false | And product "product1" type should be standard @@ -21,5 +28,10 @@ Feature: Add product image from Back Office (BO) | image reference | is cover | legend | position | | image1 | true | en-US: | 1 | | image2 | false | en-US: | 2 | - -#todo: assert image types (which is not present in dummy database), assert combination & pack images + And images "[image1, image2]" should have following types generated: + | name | width | height | + | cart_default | 125 | 125 | + | home_default | 250 | 250 | + | large_default | 800 | 800 | + | medium_default | 452 | 452 | + | small_default | 98 | 98 | From 3f49ceaa074601f23279092f80c1f9d3711eb10a Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 12:43:22 +0200 Subject: [PATCH 80/87] fix transformer --- .../Product/ProductImageFeatureContext.php | 17 ++++++++++++----- .../Features/Transform/StringArrayTransform.php | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 9cbb7fec62f81..e2057c93a7ee4 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -98,9 +98,7 @@ public function assertProductsImageTypesExists(TableNode $tableNode) } /** - * @Given /^images "\[.*?\]" should have following types generated:$/ - * - * @Transform("\[.*?\]") + * @Given images :imageReferences should have following types generated: * * @param string[] $imageReferences * @param TableNode $tableNode @@ -113,13 +111,22 @@ public function assertProductImageTypesGenerated(array $imageReferences, TableNo $imageId = $this->getSharedStorage()->get($imageReference); foreach ($dataRows as $dataRow) { $imgPath = $this->parseGeneratedImagePath($imageId, $dataRow['name']); - if (file_exists($imgPath)) { - //@todo: assert file dimensions + if (!file_exists($imgPath)) { + throw new RuntimeException(sprintf('File "%s" does not exist', $imgPath)); } + + $info = getimagesize($imgPath); + dump($info); } } } + /** + * @param int $imageId + * @param string $imageTypeName + * + * @return string + */ private function parseGeneratedImagePath(int $imageId, string $imageTypeName): string { $directories = str_split((string) $imageId); diff --git a/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php b/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php index 8e614b01b518a..93867d871a466 100644 --- a/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php +++ b/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php @@ -36,7 +36,7 @@ trait StringArrayTransform { /** - * @Transform /"\[.*?\]"$/ + * @Transform /^\[.*?\]$/ * * @param string $string * From ca62e38e037ed0448f4f9907463bbdfa737d02a7 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 12:46:48 +0200 Subject: [PATCH 81/87] fix image path building --- src/Adapter/Image/ImageGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/Image/ImageGenerator.php b/src/Adapter/Image/ImageGenerator.php index 345e9656ea130..542f9dd98d284 100644 --- a/src/Adapter/Image/ImageGenerator.php +++ b/src/Adapter/Image/ImageGenerator.php @@ -98,7 +98,7 @@ protected function resize(string $filePath, ImageType $imageType): bool return ImageManager::resize( $filePath, - sprintf('%s-%s%s', rtrim($filePath, $fileExtension), stripslashes($imageType->name), $destinationExtension), + sprintf('%s-%s%s', rtrim($filePath, '.' . $fileExtension), stripslashes($imageType->name), $destinationExtension), $width, $height, trim(mime_content_type($filePath), 'image/') From a3f22aa3f33bc8f27231c98a4818bec06aaf345f Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 13:42:02 +0200 Subject: [PATCH 82/87] assert image height and width --- .../Domain/Product/ProductImageFeatureContext.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index e2057c93a7ee4..0878ad62b42a9 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -115,8 +115,17 @@ public function assertProductImageTypesGenerated(array $imageReferences, TableNo throw new RuntimeException(sprintf('File "%s" does not exist', $imgPath)); } - $info = getimagesize($imgPath); - dump($info); + $imgInfo = getimagesize($imgPath); + Assert::assertEquals( + $dataRow['width'], + $imgInfo[0], + sprintf('Unexpected generated image "%s" width', $dataRow['name']) + ); + Assert::assertEquals( + $dataRow['height'], + $imgInfo[1], + sprintf('Unexpected generated image "%s" height', $dataRow['name']) + ); } } } @@ -132,7 +141,7 @@ private function parseGeneratedImagePath(int $imageId, string $imageTypeName): s $directories = str_split((string) $imageId); $path = implode('/', $directories); - return _PS_IMG_DIR_ . $path . '/' . $imageId . '-' . $imageTypeName . '.jpg'; + return _PS_PROD_IMG_DIR_ . $path . '/' . $imageId . '-' . $imageTypeName . '.jpg'; } /** From f40088cd011fb1484e995a26eb7b57c569bdd6d6 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 13:49:00 +0200 Subject: [PATCH 83/87] fix yml after rebase --- .../Resources/config/services/adapter/product.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml index 23f8ba2225496..f7ffe7c63b82d 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/product.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/product.yml @@ -391,7 +391,6 @@ services: arguments: - '@prestashop.adapter.product.repository.product_repository' -<<<<<<< HEAD prestashop.adapter.product.command_handler.duplicate_product_handler: class: PrestaShop\PrestaShop\Adapter\Product\CommandHandler\DuplicateProductHandler arguments: From 4852dfd84eab28d9e8bfd2b3899927e512b04917 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Tue, 17 Nov 2020 19:40:56 +0200 Subject: [PATCH 84/87] remove Transformer as it was moved into separate PR --- .../Product/ProductImageFeatureContext.php | 3 -- .../Transform/StringArrayTransform.php | 49 ------------------- 2 files changed, 52 deletions(-) delete mode 100644 tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 0878ad62b42a9..884da96b5c812 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -36,13 +36,10 @@ use PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductImage; use RuntimeException; use Tests\Integration\Behaviour\Features\Context\Util\PrimitiveUtils; -use Tests\Integration\Behaviour\Features\Transform\StringArrayTransform; use Tests\Resources\DummyFileUploader; class ProductImageFeatureContext extends AbstractProductFeatureContext { - use StringArrayTransform; - /** * @var ProductImageRepository */ diff --git a/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php b/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php deleted file mode 100644 index 93867d871a466..0000000000000 --- a/tests/Integration/Behaviour/Features/Transform/StringArrayTransform.php +++ /dev/null @@ -1,49 +0,0 @@ - - * @copyright Since 2007 PrestaShop SA and Contributors - * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) - */ - -declare(strict_types=1); - -namespace Tests\Integration\Behaviour\Features\Transform; - -use Tests\Integration\Behaviour\Features\Context\Util\PrimitiveUtils; - -/** - * Trait containing string to array | array to string transformations - */ -trait StringArrayTransform -{ - /** - * @Transform /^\[.*?\]$/ - * - * @param string $string - * - * @return array - */ - public function transformStringArrayToArray(string $string): array - { - return PrimitiveUtils::castStringArrayIntoArray($string); - } -} From 7bf45d37a3b93dc922be57ee4ad3db5b38699dda Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Wed, 18 Nov 2020 16:04:47 +0200 Subject: [PATCH 85/87] remove irrelevant todo --- .../Context/Domain/Product/ProductImageFeatureContext.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 884da96b5c812..01747356ddf40 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -59,7 +59,6 @@ public function __construct() */ public function uploadImage(string $productReference, string $imageReference, string $fileName): void { - //@todo: behats database contains empty ImageType. $pathName = DummyFileUploader::upload($fileName); $imageId = $this->getCommandBus()->handle(new AddProductImageCommand( From cee043fef5aa05c257ee68a86084282839bd6dc9 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 20 Nov 2020 11:55:34 +0200 Subject: [PATCH 86/87] fix licence --- src/Adapter/Image/Uploader/AbstractImageUploader.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Adapter/Image/Uploader/AbstractImageUploader.php b/src/Adapter/Image/Uploader/AbstractImageUploader.php index 40b6792e6148f..6405d0788f982 100644 --- a/src/Adapter/Image/Uploader/AbstractImageUploader.php +++ b/src/Adapter/Image/Uploader/AbstractImageUploader.php @@ -1,11 +1,12 @@ - * @copyright 2007-2020 PrestaShop SA and Contributors + * @author PrestaShop SA and Contributors + * @copyright Since 2007 PrestaShop SA and Contributors * @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) - * International Registered Trademark & Property of PrestaShop SA */ namespace PrestaShop\PrestaShop\Adapter\Image\Uploader; From cfd4504cd9be1b3a17c4538704f6048654a83708 Mon Sep 17 00:00:00 2001 From: Julius Zukauskas Date: Fri, 20 Nov 2020 12:00:01 +0200 Subject: [PATCH 87/87] adjust scenario wording --- .../Product/ProductImageFeatureContext.php | 38 +++++++++---------- .../Scenario/Product/add_image.feature | 4 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php index 01747356ddf40..612661e36d2e0 100644 --- a/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php +++ b/tests/Integration/Behaviour/Features/Context/Domain/Product/ProductImageFeatureContext.php @@ -50,25 +50,6 @@ public function __construct() $this->productImageRepository = $this->getContainer()->get('prestashop.adapter.product.repository.product_image_repository'); } - /** - * @When I add new product :productReference image :imageReference named :fileName - * - * @param string $productReference - * @param string $imageReference - * @param string $fileName - */ - public function uploadImage(string $productReference, string $imageReference, string $fileName): void - { - $pathName = DummyFileUploader::upload($fileName); - - $imageId = $this->getCommandBus()->handle(new AddProductImageCommand( - $this->getSharedStorage()->get($productReference), - $pathName - )); - - $this->getSharedStorage()->set($imageReference, $imageId->getValue()); - } - /** * @Given following image types should be applicable to products: */ @@ -93,6 +74,25 @@ public function assertProductsImageTypesExists(TableNode $tableNode) } } + /** + * @When I add new image :imageReference named :fileName to product :productReference + * + * @param string $imageReference + * @param string $fileName + * @param string $productReference + */ + public function uploadImage(string $imageReference, string $fileName, string $productReference): void + { + $pathName = DummyFileUploader::upload($fileName); + + $imageId = $this->getCommandBus()->handle(new AddProductImageCommand( + $this->getSharedStorage()->get($productReference), + $pathName + )); + + $this->getSharedStorage()->set($imageReference, $imageId->getValue()); + } + /** * @Given images :imageReferences should have following types generated: * diff --git a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature index 2cd7cead17de9..dfab64e4fd7f8 100644 --- a/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature +++ b/tests/Integration/Behaviour/Features/Scenario/Product/add_image.feature @@ -19,11 +19,11 @@ Feature: Add product image from Back Office (BO) | is_virtual | false | And product "product1" type should be standard And product "product1" should have no images - When I add new product "product1" image "image1" named "app_icon.png" + When I add new image "image1" named "app_icon.png" to product "product1" Then product "product1" should have following images: | image reference | is cover | legend | position | | image1 | true | en-US: | 1 | - When I add new product "product1" image "image2" named "logo.jpg" + When I add new image "image2" named "logo.jpg" to product "product1" Then product "product1" should have following images: | image reference | is cover | legend | position | | image1 | true | en-US: | 1 |