search

4rn0 / statamic-v2-image-optimizer

Statamic v2 Addon to optimizes new Asset and Glide images
https://statamic.com/marketplace/addons/imageoptimizer
4 stars 2 forks source link

Doesn't work with assets on S3 #11

Open madsem opened 5 years ago

madsem commented 5 years ago

When uploading assets in a container that uses the S3 driver, the assets are logged as successfully optimized, but never actually are.

I assume we should use either Guzzle or Curl, to download the asset if a container is S3 enabled, then optimizing it inside the tmp dir, and then moving/uploading it back to S3.

madsem commented 5 years ago

Actually no, not Guzzle/Curl. The way this should work so it’s compatible with all asset container drivers is to use the File Facade imho.

So assets should be moved only using ‘File’ and not by the built in optimizer libraries.

I think in general assets should be moved to tmp dir, then the optimizer library optimizes it there and copies it back to tmp, and then using the File Facade it has to be moved back to the asset container.

That way I believe S3 should also work automatically.

madsem commented 5 years ago

The more I think about it, the more I feel the addon should have an API for this? The problem when using a cloud driver is, that not only would the ImageOptimizer download the asset that was just uploaded, and then optimize it and upload it again.

It would also connect to s3 for every filesize & lastmodified check...

If there was an API one could trigger the optimization when needed, and only optimize it on Glide events so that local copies are optimized. But for AssetUploaded events do nothing, and instead, make the optimization available through API.

Then again, on the other hand, as all these extra calls would only be made whenever an asset is newly uploaded, that might be okay as it doesn't happen on page load... And Glide cache would still be local even if assets are on s3, so still fast

Thoughts?

4rn0 commented 5 years ago

Hey man!

assets should be moved to tmp dir, then the optimizer library optimizes it there and copies it back to tmp, and then using the File Facade it has to be moved back to the asset container.

I think this would be the easiest way to get this working with S3 and other Flysystem drivers. But optimising the Glide manipulations is the most important thing here and I think the Glide cache is always on the local filesystem?

madsem commented 5 years ago

Yes I think so too, Glide cache is always local, and the additional checks for filesize + lastmodified date, can all be done via the File facade, that would make it all compatible with all Flysystem drivers. https://docs.statamic.com/addons/api/api-file

4rn0 commented 5 years ago

I'll set up a branch and implement some of the Flysystem stuff. I'm a bit swamped with client work right now, but I'll hopefully get something done this week!

madsem commented 5 years ago

Ok I'll probably send a pull request, as I'm implementing the flysystem stuff in a custom addon atm, then we can just look it over and you can merge it in after changes.