Skip to content

Commit

Permalink
ENGCOM-8252: Product images are being duplicated on import #26713
Browse files Browse the repository at this point in the history
  • Loading branch information
@sidolov
sidolov authored Oct 8, 2020
2 parents d31cd74 + e1f7557 commit c9fbde1
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 39 deletions.
162 changes: 128 additions & 34 deletions app/code/Magento/CatalogImportExport/Model/Import/Product.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\Filesystem;
use Magento\Framework\Filesystem\Driver\File;
use Magento\Framework\Intl\DateTimeFactory;
use Magento\Framework\Model\ResourceModel\Db\ObjectRelationProcessor;
use Magento\Framework\Model\ResourceModel\Db\TransactionManagerInterface;
Expand All @@ -44,9 +45,10 @@
* @SuppressWarnings(PHPMD.ExcessivePublicCount)
* @since 100.0.2
*/
class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity
class Product extends AbstractEntity
{
const CONFIG_KEY_PRODUCT_TYPES = 'global/importexport/import_product_types';
public const CONFIG_KEY_PRODUCT_TYPES = 'global/importexport/import_product_types';
private const HASH_ALGORITHM = 'sha256';

/**
* Size of bunch - part of products to save in one step.
Expand Down Expand Up @@ -766,6 +768,11 @@ class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity
*/
private $linkProcessor;

/**
* @var File
*/
private $fileDriver;

/**
* @param \Magento\Framework\Json\Helper\Data $jsonHelper
* @param \Magento\ImportExport\Helper\Data $importExportData
Expand Down Expand Up @@ -814,6 +821,7 @@ class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity
* @param StatusProcessor|null $statusProcessor
* @param StockProcessor|null $stockProcessor
* @param LinkProcessor|null $linkProcessor
* @param File|null $fileDriver
* @throws LocalizedException
* @throws \Magento\Framework\Exception\FileSystemException
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
Expand Down Expand Up @@ -866,7 +874,8 @@ public function __construct(
ProductRepositoryInterface $productRepository = null,
StatusProcessor $statusProcessor = null,
StockProcessor $stockProcessor = null,
LinkProcessor $linkProcessor = null
LinkProcessor $linkProcessor = null,
?File $fileDriver = null
) {
$this->_eventManager = $eventManager;
$this->stockRegistry = $stockRegistry;
Expand Down Expand Up @@ -930,6 +939,7 @@ public function __construct(
$this->dateTimeFactory = $dateTimeFactory ?? ObjectManager::getInstance()->get(DateTimeFactory::class);
$this->productRepository = $productRepository ?? ObjectManager::getInstance()
->get(ProductRepositoryInterface::class);
$this->fileDriver = $fileDriver ?: ObjectManager::getInstance()->get(File::class);
}

/**
Expand Down Expand Up @@ -1570,7 +1580,10 @@ protected function _saveProducts()
$uploadedImages = [];
$previousType = null;
$prevAttributeSet = null;

$importDir = $this->_mediaDirectory->getAbsolutePath($this->getUploader()->getTmpDir());
$existingImages = $this->getExistingImages($bunch);
$this->addImageHashes($existingImages);

foreach ($bunch as $rowNum => $rowData) {
// reset category processor's failed categories array
Expand Down Expand Up @@ -1738,7 +1751,8 @@ protected function _saveProducts()
$position = 0;
foreach ($rowImages as $column => $columnImages) {
foreach ($columnImages as $columnImageKey => $columnImage) {
if (!isset($uploadedImages[$columnImage])) {
$uploadedFile = $this->getAlreadyExistedImage($rowExistingImages, $columnImage, $importDir);
if (!$uploadedFile && !isset($uploadedImages[$columnImage])) {
$uploadedFile = $this->uploadMediaFiles($columnImage);
$uploadedFile = $uploadedFile ?: $this->getSystemFile($columnImage);
if ($uploadedFile) {
Expand All @@ -1753,7 +1767,7 @@ protected function _saveProducts()
ProcessingError::ERROR_LEVEL_NOT_CRITICAL
);
}
} else {
} elseif (isset($uploadedImages[$columnImage])) {
$uploadedFile = $uploadedImages[$columnImage];
}

Expand Down Expand Up @@ -1782,8 +1796,7 @@ protected function _saveProducts()
}

if (isset($rowLabels[$column][$columnImageKey])
&& $rowLabels[$column][$columnImageKey] !=
$currentFileData['label']
&& $rowLabels[$column][$columnImageKey] !== $currentFileData['label']
) {
$labelsForUpdate[] = [
'label' => $rowLabels[$column][$columnImageKey],
Expand All @@ -1792,7 +1805,7 @@ protected function _saveProducts()
];
}
} else {
if ($column == self::COL_MEDIA_IMAGE) {
if ($column === self::COL_MEDIA_IMAGE) {
$rowData[$column][] = $uploadedFile;
}
$mediaGallery[$storeId][$rowSku][$uploadedFile] = [
Expand Down Expand Up @@ -1908,24 +1921,14 @@ protected function _saveProducts()
}
}

$this->saveProductEntity(
$entityRowsIn,
$entityRowsUp
)->_saveProductWebsites(
$this->websitesCache
)->_saveProductCategories(
$this->categoriesCache
)->_saveProductTierPrices(
$tierPrices
)->_saveMediaGallery(
$mediaGallery
)->_saveProductAttributes(
$attributes
)->updateMediaGalleryVisibility(
$imagesForChangeVisibility
)->updateMediaGalleryLabels(
$labelsForUpdate
);
$this->saveProductEntity($entityRowsIn, $entityRowsUp)
->_saveProductWebsites($this->websitesCache)
->_saveProductCategories($this->categoriesCache)
->_saveProductTierPrices($tierPrices)
->_saveMediaGallery($mediaGallery)
->_saveProductAttributes($attributes)
->updateMediaGalleryVisibility($imagesForChangeVisibility)
->updateMediaGalleryLabels($labelsForUpdate);

$this->_eventManager->dispatch(
'catalog_product_import_bunch_save_after',
Expand All @@ -1939,6 +1942,87 @@ protected function _saveProducts()

// phpcs:enable

/**
* Returns image hash by path
*
* @param string $path
* @return string
*/
private function getFileHash(string $path): string
{
return hash_file(self::HASH_ALGORITHM, $path);
}

/**
* Returns existed image
*
* @param array $imageRow
* @param string $columnImage
* @param string $importDir
* @return string
*/
private function getAlreadyExistedImage(array $imageRow, string $columnImage, string $importDir): string
{
if (filter_var($columnImage, FILTER_VALIDATE_URL)) {
$hash = $this->getFileHash($columnImage);
} else {
$path = $importDir . DS . $columnImage;
$hash = $this->isFileExists($path) ? $this->getFileHash($path) : '';
}

return array_reduce(
$imageRow,
function ($exists, $file) use ($hash) {
if (!$exists && isset($file['hash']) && $file['hash'] === $hash) {
return $file['value'];
}

return $exists;
},
''
);
}

/**
* Generate hashes for existing images for comparison with newly uploaded images.
*
* @param array $images
* @return void
*/
private function addImageHashes(array &$images): void
{
$productMediaPath = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA)
->getAbsolutePath(DS . 'catalog' . DS . 'product');

foreach ($images as $storeId => $skus) {
foreach ($skus as $sku => $files) {
foreach ($files as $path => $file) {
if ($this->fileDriver->isExists($productMediaPath . $file['value'])) {
$fileName = $productMediaPath . $file['value'];
$images[$storeId][$sku][$path]['hash'] = $this->getFileHash($fileName);
}
}
}
}
}

/**
* Is file exists
*
* @param string $path
* @return bool
*/
private function isFileExists(string $path): bool
{
try {
$fileExists = $this->fileDriver->isExists($path);
} catch (\Exception $exception) {
$fileExists = false;
}

return $fileExists;
}

/**
* Clears entries from Image Set and Row Data marked as no_selection
*
Expand All @@ -1950,9 +2034,8 @@ private function clearNoSelectionImages($rowImages, $rowData)
{
foreach ($rowImages as $column => $columnImages) {
foreach ($columnImages as $key => $image) {
if ($image == 'no_selection') {
unset($rowImages[$column][$key]);
unset($rowData[$column]);
if ($image === 'no_selection') {
unset($rowImages[$column][$key], $rowData[$column]);
}
}
}
Expand Down Expand Up @@ -2095,6 +2178,21 @@ protected function _saveProductTierPrices(array $tierPriceData)
return $this;
}

/**
* Returns the import directory if specified or a default import directory (media/import).
*
* @return string
*/
private function getImportDir(): string
{
$dirConfig = DirectoryList::getDefaultConfig();
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];

return empty($this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR])
? $dirAddon . DS . $this->_mediaDirectory->getRelativePath('import')
: $this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR];
}

/**
* Returns an object for upload a media files
*
Expand All @@ -2111,11 +2209,7 @@ protected function _getUploader()
$dirConfig = DirectoryList::getDefaultConfig();
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];

if (!empty($this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR])) {
$tmpPath = $this->_parameters[Import::FIELD_NAME_IMG_FILE_DIR];
} else {
$tmpPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath('import');
}
$tmpPath = $this->getImportDir();

if (!$fileUploader->setTmpDir($tmpPath)) {
throw new LocalizedException(
Expand Down
33 changes: 28 additions & 5 deletions dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use Magento\Store\Model\Store;
use Magento\Store\Model\StoreManagerInterface;
use Magento\TestFramework\Helper\Bootstrap as BootstrapHelper;
use Magento\TestFramework\Indexer\TestCase;
use Magento\UrlRewrite\Model\ResourceModel\UrlRewriteCollection;
use Psr\Log\LoggerInterface;

Expand All @@ -50,8 +51,10 @@
* @SuppressWarnings(PHPMD.ExcessivePublicCount)
* phpcs:disable Generic.PHP.NoSilencedErrors, Generic.Metrics.NestingLevel, Magento2.Functions.StaticFunction
*/
class ProductTest extends \Magento\TestFramework\Indexer\TestCase
class ProductTest extends TestCase
{
private const LONG_FILE_NAME_IMAGE = 'magento_long_image_name_magento_long_image_name_magento_long_image_name.jpg';

/**
* @var \Magento\CatalogImportExport\Model\Import\Product
*/
Expand Down Expand Up @@ -1029,13 +1032,12 @@ function (\Magento\Framework\DataObject $item) {
)
);

$this->importDataForMediaTest('import_media_additional_images.csv');
$this->importDataForMediaTest('import_media_additional_long_name_image.csv');
$product->cleanModelCache();
$product = $this->getProductBySku('simple_new');
$items = array_values($product->getMediaGalleryImages()->getItems());
$images[] = ['file' => '/m/a/magento_additional_image_three.jpg', 'label' => ''];
$images[] = ['file' => '/m/a/magento_additional_image_four.jpg', 'label' => ''];
$this->assertCount(7, $items);
$images[] = ['file' => '/m/a/' . self::LONG_FILE_NAME_IMAGE, 'label' => ''];
$this->assertCount(6, $items);
$this->assertEquals(
$images,
array_map(
Expand All @@ -1047,6 +1049,23 @@ function (\Magento\Framework\DataObject $item) {
);
}

/**
* Test import twice and check that image will not be duplicate
*
* @magentoDataFixture mediaImportImageFixture
* @return void
*/
public function testSaveMediaImageDuplicateImages(): void
{
$this->importDataForMediaTest('import_media.csv');
$imagesCount = count($this->getProductBySku('simple_new')->getMediaGalleryImages()->getItems());

// import the same file again
$this->importDataForMediaTest('import_media.csv');

$this->assertCount($imagesCount, $this->getProductBySku('simple_new')->getMediaGalleryImages()->getItems());
}

/**
* Test that errors occurred during importing images are logged.
*
Expand Down Expand Up @@ -1089,6 +1108,10 @@ public static function mediaImportImageFixture()
'source' => __DIR__ . '/../../../../Magento/Catalog/_files/magento_thumbnail.jpg',
'dest' => $dirPath . '/magento_thumbnail.jpg',
],
[
'source' => __DIR__ . '/../../../../Magento/Catalog/_files/' . self::LONG_FILE_NAME_IMAGE,
'dest' => $dirPath . '/' . self::LONG_FILE_NAME_IMAGE,
],
[
'source' => __DIR__ . '/_files/magento_additional_image_one.jpg',
'dest' => $dirPath . '/magento_additional_image_one.jpg',
Expand Down
2 changes: 2 additions & 0 deletions ...gento/CatalogImportExport/Model/Import/_files/import_media_additional_long_name_image.csv
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
sku,additional_images
simple_new,magento_long_image_name_magento_long_image_name_magento_long_image_name.jpg

0 comments on commit c9fbde1

Please sign in to comment.