Duplicating product copies product images couple of hundred times

[qbixx]

When duplicating a product to create a similar product all the required information is copied over. This also includes the product images. However when saving the new (duplicated) product, it takes quite a long time before the product get's saved. This is because the attached images are copied a few hundred times on the server.

The amount of copies is not always the same but ranges from about 200 to 2000 copies. The copies are all in the same folder that the original images resides and will have a format of e.g. [image]_100.jpg.

Preconditions

I've tested this both on Magento 2.1.4 and 2.1.6.
PHP 7.0.18 and MariaDB 10.1.22

Steps to reproduce

1. Create a new product and fill out the required fields. Also add an image
2. Save the product and click on the right side on 'Save & Duplicate'
3. Save the duplicated product
4. Open original product, 'Save & Duplicate' it.

Expected result

The image(s) on the duplicated product should be copied once.

Actual result

The image(s) on the duplicated product are copied multiple times.

[ZAV79]

I am also experiencing this - but to a somewhat different level.
When one of my users duplicates a product it seems that as above each image is duplicated multiple times, but it does each image in the entire catalog, not just the product being duplicated.

This means that duplicating just a couple of products can create 100000s images.
This issue was not realised until a 2TB disk had been eaten up.

We are using Magento 2.1.15, PHP 7.0.18, mysql 5.0.12 , Apache/2.4.6, varnish

Unfortunately because the products are not duplicated often, we don't know when this bug was introduced.

We suspect it is something to do with products that were imported using the migration tool from 1.9 as the error does not seem to happen with products added to the catalog after the data migration

[JasonScottM]

Magento ver. 2.1.5 BUG "multiple image generation on product copy".

this is down to a particularly bad piect of code in vendor\magento\module-catalog\Model\Product\Copier.php that loops infinately is the save fails due to an AlreadyExistsException. unfortunately data issues in the url rewrite table can cause this failure and as Magento devs failed to put in place a decent cleanup process to remove immages on save fail you get an ever increasing set untill php times out.

public function copy(\Magento\Catalog\Model\Product $product)
{
................
do {
$urlKey = $duplicate->getUrlKey();
$urlKey = preg_match('/(.*)-(\d+)$/', $urlKey, $matches)
? $matches[1] . '-' . ($matches[2] + 1)
: $urlKey . '-1';
$duplicate->setUrlKey($urlKey);
try {
$duplicate->save();
$isDuplicateSaved = true;
} catch (\Magento\Framework\Exception\AlreadyExistsException $e) {
}
} while (!$isDuplicateSaved);
...............
}

luckily there is code in vendor\magento\module-catalog-url-rewrite\observer\ProductProcessUrlRewriteSavingObserver.php that is a life saver

public function execute(\Magento\Framework\Event\Observer $observer)
{
.................................
if ($product->isVisibleInSiteVisibility()) {
$this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product));
}
.................................
}

so by inserting

$duplicate->setVisibility(\Magento\Catalog\Model\Product\Visibility::VISIBILITY_NOT_VISIBLE);

into the object being saved the url rewrites will not be generated and the duplicate will save without the multiple image loop (though multiples will still occur if its been copied n times previously but only n copies)

magento devs really need to sort this out, relying on the save excepting rather than first looking up a free rewrite is an error, The code should lookup a free url against the url_key attribute values in catalog_product_entity_varchar first then attempt the save once only and throw the exception if it fails, PLEASE stop putting empty catch staements in loops !!!

my quick fix to correct this error is to insert $duplicate->setVisibility(\Magento\Catalog\Model\Product\Visibility::VISIBILITY_NOT_VISIBLE); in the core copy function which then reads:-

public function copy(\Magento\Catalog\Model\Product $product)
{
$product->getWebsiteIds();
$product->getCategoryIds();

    /** @var \Magento\Catalog\Model\Product $duplicate */
    $duplicate = $this->productFactory->create();
    $duplicate->setData($product->getData());
    $duplicate->setOptions([]);
    $duplicate->setIsDuplicate(true);
    $duplicate->setOriginalId($product->getEntityId());
    $duplicate->setStatus(\Magento\Catalog\Model\Product\Attribute\Source\Status::STATUS_DISABLED);
    $duplicate->setCreatedAt(null);
    $duplicate->setUpdatedAt(null);
    $duplicate->setId(null);
    $duplicate->setStoreId(\Magento\Store\Model\Store::DEFAULT_STORE_ID);
    $duplicate->setVisibility(\Magento\Catalog\Model\Product\Visibility::VISIBILITY_NOT_VISIBLE);    
    $this->copyConstructor->build($product, $duplicate);
    $isDuplicateSaved = false;
    do {
        $urlKey = $duplicate->getUrlKey();
        $urlKey = preg_match('/(.*)-(\d+)$/', $urlKey, $matches)
            ? $matches[1] . '-' . ($matches[2] + 1)
            : $urlKey . '-1';
        $duplicate->setUrlKey($urlKey);
        try {
            $duplicate->save();
            $isDuplicateSaved = true;
        } catch (\Magento\Framework\Exception\AlreadyExistsException $e) {
        }
    } while (!$isDuplicateSaved);
    $this->getOptionRepository()->duplicate($product, $duplicate);
    $metadata = $this->getMetadataPool()->getMetadata(ProductInterface::class);
    $product->getResource()->duplicate(
        $product->getData($metadata->getLinkField()),
        $duplicate->getData($metadata->getLinkField())
    );
    return $duplicate;
}

you will also note from the origional Magento code that if your URL keys / Sku convention is latter half numeric with a - in to ie AAAAA-00001 then prepare for a long wait on product copies or change the - in the logic to an _ if you dont use _ in your standard url keys.

[pascaladriaansen]

I think this is the same issue as #4096.

[veloraven]

Closed as a duplicate of #4096

[qbixx]

As far as I can see this isn't a duplicate of #4096.

When looking at the behaviour of #4096 there are some different things going on than in this issue.

When duplicating a product on this issue the page doesn't go in a loop to reload the product page. Also, with this issue the website doesn't become unavailable. The website itself is still working, only the disk space of the server is filling up with duplicate product images.

[jonahellison]

Confirmed this is still an issue on 2.1.8 for migrated products. The "setVisibility" fix from JasonScottM did work.

[vforvalerio87]

Is there any way to safely and reliably delete all duplicates for all products? Thanks

[JasonScottM]

If you are asking is there a way to delete all unused duplicate images? then yes, attached is a script I put together for that task, if your database has a prefix set you may need to change the table name. run the script and if you are happy the red results shown are the ones to delete, uncomment the 2 unlink lines are re-run. To be sure you can recover, I suggest backing up the pub/media/catalog Folder first. From: Valerio Versace [mailto:notifications@github.com] Sent: 14 September 2017 17:45 To: magento/magento2 <magento2@noreply.github.com> Cc: JasonScottM <jason.mummery@micommerce.com>; Comment <comment@noreply.github.com> Subject: Re: [magento/magento2] Duplicating product copies product images couple of hundred times (#9466) Is there any way to safely and reliably delete all duplicates for all products? Thanks — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#9466 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AOY7R-jzSKtkE6I3vx29lKr-IVYmZcC3ks5siVgVgaJpZM4NNQ8J> . <https://github.com/notifications/beacon/AOY7R2kofUTbkZwpqO_eqlJn38NFrGXmks5siVgVgaJpZM4NNQ8J.gif>

[hostep]

God damned, we just ran into this today on a Magento 2.1.9 installation. 12 GB of diskspace eaten away in a 40 minutes by duplicating some products until our server crashed with no space left.

Would be great if this got fixed!

[hostep]

@magento-engcom-team: can this ticket get re-opened? It has the label G1 Passed while the similar looking issue #4096 has G1 Failed.
Thanks!

[hostep]

So the exact steps to reproduce are outlined below, because following the steps from @qbixx on a clean installation isn't going to trigger the problem:

Steps to reproduce

1. Set up a clean Magento 2.1.9 installation
2. Create a simple product, assign it an image 'pic.jpg' and save
3. Look at the image directory, it contains 1 image:

$ ls -1 pub/media/catalog/product/p/i/
pic.jpg

4. Save and duplicate the product
5. Look at the image directory, it contains 2 images (so far so good):

$ ls -1 pub/media/catalog/product/p/i/
pic.jpg
pic_1.jpg

6. Open the first product you created and choose to save and duplicate it again
7. Look at the image directory, it contains 4 images (uh oh, something's not right here):

$ ls -1 pub/media/catalog/product/p/i/
pic.jpg
pic_1.jpg
pic_2.jpg
pic_3.jpg

8. Open the first product you created and choose to save and duplicate it again
9. Look at the image directory, it contains 7 images (this is starting to get out of control):

$ ls -1 pub/media/catalog/product/p/i/
pic.jpg
pic_1.jpg
pic_2.jpg
pic_3.jpg
pic_4.jpg
pic_5.jpg
pic_6.jpg

...

Expected result

After every duplication, only one new image file is being created

Further discoveries

It no longer seems to be reproducible in Magento 2.2.0
This has part to do with the fact that if a duplicate url key is encountered, it now throws a different exception, instead of \Magento\Framework\Exception\AlreadyExistsException the exception Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException is being thrown, and this one isn't being catched in https://github.com/magento/magento2/blob/2.2.0/app/code/Magento/Catalog/Model/Product/Copier.php#L85
So it jumps out of that loop.
Interestingly, if I change the exception to be catched in 2.2.0, the problem still no longer occurs, so something else must have changed next to that different exception being thrown. If somebody knows what this could have been, that might be the real fix...

And I agree with @JasonScottM:

magento devs really need to sort this out, relying on the save excepting rather than first looking up a free rewrite is an error, The code should lookup a free url against the url_key attribute values in catalog_product_entity_varchar first then attempt the save once only and throw the exception if it fails, PLEASE stop putting empty catch staements in loops !!!

Trying to save a model and try to save it multiple times in a loop until an exception no longer is being thrown is a really bad idea.