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

Transforms don't get deleted on remote volumes #341

Closed
JensvdHeydt opened this issue May 30, 2022 · 18 comments
Closed

Transforms don't get deleted on remote volumes #341

JensvdHeydt opened this issue May 30, 2022 · 18 comments
Labels
bug need info Need more information on the issue

Comments

@JensvdHeydt
Copy link

Describe the bug

I'm using DigitalOcean Spaces Volume v1.1.3 and when I delete an asset some directories and files don't get removed from the remote volume. It seems to be the webp variants that image optimize creates and their respective directories. The original file gets deleted by craft. There's a comment in the executed funtion in Optimize.php, line 397: "// Only do this for local Craft transforms". Maybe that's the issue?

To reproduce

Steps to reproduce the behaviour:

  1. When using a remote volume upload an asset and let craft create optimized variants
  2. Delete the asset
  3. Transform variants (webp) don't get deleted from remote volume.

Expected behaviour

Remote volume should not be ignored when deleting transform variant files.

Screenshots

Bildschirmfoto 2022-05-30 um 11 25 28

Versions

  • Plugin version: 1.6.44
  • Craft version: 3.7.43
@JensvdHeydt
Copy link
Author

@khalwat is this issue something you would consider as a bug? It would be wonderful if you could give me a short feedback. Otherwise I would have to find other ways of fixing this issue with my own event handlers.

@khalwat
Copy link
Contributor

khalwat commented Jun 14, 2022

i think it is likely a bug. I'll be looking into it!

@khalwat
Copy link
Contributor

khalwat commented Jun 27, 2022

We've tried to reproduce this, but are unable to -- could it perhaps be something specific to Digital Ocean Spaces? ImageOptimize doesn't actually do anything any different here than regular old Craft transforms.

So I'd expect the same behavior if you did a Craft transform in your templates, and then deleted the original image.

@khalwat khalwat added the need info Need more information on the issue label Jun 27, 2022
@JensvdHeydt
Copy link
Author

JensvdHeydt commented Jun 28, 2022

Hi @khalwat,

thanks for the feedback. I'll verify if the bug also exists using craft transforms. I'll get back to you.

@JensvdHeydt
Copy link
Author

@khalwat, I've tested with an ordinary craft transform and the generated assets don't get deleted on digital ocean. Could you point me in the right direction how I would be able to solve this issue, maybe with my own plugin? I guess there's an event I could listen for?

@JensvdHeydt
Copy link
Author

JensvdHeydt commented Jul 6, 2022

@khalwat I tested this once more with a local volume and found the same behaviour:

  • ImageOptimize creates 3 image variants for my PNG with different image sizes
  • It also creates 3 WEBP named something like "original name.png.webp" variants
  • When I delete the image from the local asset volume it deletes the PNG transforms of different size but leaves the *.png.WEBPs that were also created

Did this work at your end?

@khalwat
Copy link
Contributor

khalwat commented Jul 6, 2022

I will give it a whirl, but I've not encountered this before, no.

@JensvdHeydt
Copy link
Author

@khalwat

I've debugged some more and found something interesting:

  • If I edit my Image Optimizer field and set the image format to JPG for example, all my transforms get deleted correctly
  • If I set the Image Optimizer field's image format to "auto" the webp - files remain in undeleted

I think the problem is in Optimize.php, line 799, function cleanupImageVariants. $fileformat is always empty, when I am using "auto" as image format. Therefore the function to delete the transforms is never fully executed
Bildschirmfoto 2022-07-07 um 14 39 52

@curtismorte
Copy link

Could this potentially be related to the issues in #329?

Specifically, the issue there was caused by the format not being set when the image format is "auto".

@JensvdHeydt
Copy link
Author

Yes, I think it's related or at least has a similar cause. If the format is empty the cleanup function never actually deletes transform files, @curtismorte

empty($activeImageVariantCreators[$fileFormat]) is always false in that case.

@khalwat
Copy link
Contributor

khalwat commented Jul 11, 2022

@JensvdHeydt nice sleuthing! Sorry for being slow to respond, on a holiday, but let me roll out a fix.

khalwat added a commit that referenced this issue Jul 11, 2022
khalwat added a commit that referenced this issue Jul 11, 2022
@khalwat
Copy link
Contributor

khalwat commented Jul 11, 2022

Addressed in: d71a87a & 9061074

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-imageoptimize": "dev-develop as 1.6.48”,

Then do a composer clear-cache && composer update

Craft CMS 4:

…..

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-imageoptimize": "dev-develop-v4 as 4.0.2”,

Then do a composer clear-cache && composer update

@JensvdHeydt
Copy link
Author

@khalwat thanks! I'll give it a spin.

@JensvdHeydt
Copy link
Author

@khalwat Just tested it on a local and remote volume. Both worked. Great work and never mind those "slow" responses on a holiday!

@JensvdHeydt
Copy link
Author

JensvdHeydt commented Jul 13, 2022

Hi @khalwat,
one last thing that we noticed: There seems to be a problem when using uppercase letters in the file extension, like ".PNG" or ".JPG". In that case, transforms don't get deleted. Maybe you could implement strtolower before comparing the filetypes?

@khalwat
Copy link
Contributor

khalwat commented Jul 13, 2022

@JensvdHeydt Just addressed that now

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-imageoptimize": "dev-develop as 1.6.48”,

Then do a composer clear-cache && composer update

Craft CMS 4:

…..

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-imageoptimize": "dev-develop-v4 as 4.0.2”,

Then do a composer clear-cache && composer update

@khalwat
Copy link
Contributor

khalwat commented Jul 14, 2022

@JensvdHeydt lmk if that fix does it for you, and I will close this out and cut a release.

@JensvdHeydt
Copy link
Author

@khalwat Just tested it with a remote volume. Works like it should! Thanks again.

@khalwat khalwat closed this as completed Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug need info Need more information on the issue
Projects
None yet
Development

No branches or pull requests

3 participants