aimeos / ai-controller-jobs

Aimeos e-commerce job controllers
https://aimeos.org
GNU Lesser General Public License v3.0
112 stars 17 forks source link

[Bug] Image scale job removes copied product's images #16

Closed nvindice closed 5 years ago

nvindice commented 5 years ago

Affected version: 2018.x

When we copy a product and keep all images, and run the scale job afterwards, the product images in all of the copies are removed.

Thanks in advance for fixing this issue!

aimeos commented 5 years ago

Can you be a bit more specific please? The job controller don't do much besides calling this scale() method: https://github.com/aimeos/aimeos-core/blob/master/controller/common/src/Controller/Common/Media/Standard.php#L124

The media item itself isn't removed, only it's url and preview links replaced. Can you check in the code where the problem happens you are suffering from?

nvindice commented 5 years ago

The storeFile() method will remove the old item from the file system: https://github.com/aimeos/aimeos-core/blob/95211e74ba2084f0cbd3e154ece6fc6ac1883e0f/controller/common/src/Controller/Common/Media/Standard.php#L524

When a product is duplicated in the backend, the same (!) image will be used for the duplicate. So when the original product image gets resized (and therefore deleted from the file system), the copy's image is inaccessible.

I think the cleanest way to solve this problem would be to copy all the document media items instead of just copying their reference..?

EDIT: The same problem might occur when deleting media (not only images) from a product's copy. In our case, multiple PDFs were just inaccessable one day without any reason. Because these were most likely just references to the original PDF, maybe the original product was deleted in the meantime and so all the references became dead.

aimeos commented 5 years ago

Regarding the scaling issue I think we found a better solution: In-place scaling The file names aren't changed when rescaling images so it will work with already copied media items: https://github.com/aimeos/aimeos-core/commit/2a7b0c2a17d79ea156f16433c1a2549399266519

nvindice commented 5 years ago

Looks good! I will try to adapt this to 2018.x later.

What about the delete issue? We could search the database for the filename before removing a file from the fs. This will not happen very often, so the performance should not be the biggest problem here. Unfortunately, I am not yet familiar with aimeos low-level operations.

aimeos commented 5 years ago

The copy/delete problem was fixed in one of the previous releases. Nevertheless, you should check if again to be sure it's also fixed for you.

nvindice commented 5 years ago

It's not fixed for us. Steps to reproduce:

  1. Create a product A, add an image and another media file (PDF)
  2. Copy product A -> product B. Leave the image and pdf as they are.
  3. Delete product B
  4. Product A has no image, no pdf, because the files were deleted. The references are still there.
aimeos commented 5 years ago

Problem has been fixed with this commit: https://github.com/aimeos/aimeos-core/commit/63c85379ad219d1215590cca817b587fdeabf960

nvindice commented 5 years ago

Looks good! I will test it with the next 2018.x release.