Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Product images are being duplicated on import #26713

Merged
merged 12 commits into from
Oct 13, 2020
162 changes: 128 additions & 34 deletions app/code/Magento/CatalogImportExport/Model/Import/Product.php
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
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
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