cdowdy / boltbetterthumbs

Bolt Extension for thumbnails, srcset and picture element using Glide
6 stars 2 forks source link

Image not found if request caching true in config.yml #15

Closed saltpeanuts closed 7 years ago

saltpeanuts commented 7 years ago

If request: true is set in the caching section of bolts config.yml the srcset images are not found.

The images are generated and cached. The srcset is constructed properly and it seems the paths are correct but the images don't load.

I must have set request to true by accident at some point in the past, because personally I don't need it. Still, it took me a few hours of head scratching to figure out why things weren't working properly.

cdowdy commented 7 years ago

Thanks @saltpeanuts I can reproduce this..

I think I can fix this since it seems Symfony's Reverse Proxy keeps the image url from actually hitting the PHP process.. ( I may be off here I'll have to look better haha) ...

I could make a check to see if request caching is enabled and if so make the image and serve it from the cache location. So instead of urls like:

http://example.com/img/example-image.jpg?w=400&h=225&s=4075c71f58b1aa3e332dd5eec0ef7240

you'd get

http://example.com/files/.cache/example-image.jpg/b74313aa82ec4be5e3c29b66e40b07ed.jpg

But I don't know how people would feel about that having two possible different urls for the same image (SEO for images comes to mind) if and only if request caching is enabled. The other side to that is the person would have to enable the request caching so they would be aware as long as I made the appropriate documentation additions to the config file and the docs..

Upside to this is that these files if they exist will be served from disk and or memory and be a little faster.

Downsides besides the above mentioned url is that the actual image if you were to download it wouldn't be named correctly. If you were to download the image, instead of being example-image.jpg it would end up being the hashed filename -> b74313aa82ec4be5e3c29b66e40b07ed.jpg but that is minor in my opinion haha...

I could also move the cache directory outside of the files directory to an /img directory - which is something I really don't want to do haha. That way all image urls would be /img/image-here.jpg? but that too comes with other issues I didnt' want to tackle (filesystem permissions basically... having a user create a folder on install or having the extension create the directory on install, files is almost always writeable by the server)

What do you think? Would you be ok with a different URL and the downloaded filename difference? I know statamic does something like this with vanity filenames -> https://docs.statamic.com/tags/glide#vanity-filenames

saltpeanuts commented 7 years ago

Hard to say, it'll probably bother someone, haha. Moving the cache directory outside /files sounds the most consistent, but then I don't have to make it happen.

I wonder how much of a edge case this is though? Like I said, personally I don't actually need Bolt's request cacheing, but somehow managed to turn it on by accident. The main problem was figuring out why the extension wasn't working on that site.

I've never seen any perceivable difference in performance with Bolt's request caching turned on anyway. Maybe that'll change in the future.

I'm sure I read somewhere that there are more changes in the pipes for Bolt's cacheing mechanisms, too.

Maybe a troubleshooting section in the docs would do for now, to let users know to check the setting if the extension refuses to work for them.

cdowdy commented 7 years ago

yeah I could move the cache directory outside into it's own directory but that comes with the problems above with file permissions and since this is in a public directory I can't be sure there won't be a directory called img/ which is a round about reason as to why I chose files/.cache/ for the image cache haha.

The filenames then would be the same for a regular request and ..

example.com/img/example-image.jpg/b74313aa82ec4be5e3c29b66e40b07ed.jpg

for one with request caching on.

As for if there is a difference with request caching on. In a very non scientific test haha using an Ubuntu 16.04 / Nginx/ PHP7 vagrant box, bolt's default theme and one image using this extension there's a pretty noticeable effect.

Average of doing 3 runs on a cable connection and the DOMDocumentLoaded is fired in:

request caching off ~ 3.76s request caching on: ~ 1.36s

now these aren't numbers to base anything off and doing other things in bolt instead of the regular default homepage and actually optimizing the theme since it's not very performant could lead to different numbers - which if I get time I should make a really optimized default base theme :)

But seeing these numbers Symfony's Reverse Proxy cache does a pretty good job at serving things already in the cache so I think I'll move to serving those images from the cache directory in files and if someone comes along and say's "hey this isn't working its breaking {{ something }} " then I'll change things around.

As of right now I'll leave the cache in /files/.cache/ since moving it will also have to have people remove/delete images from the old cache.

I really appreciate you bringing this to my attention @saltpeanuts ! Anything I can do to make things faster Im all for! plus it gave me a nice break from getting this extensions cache primer working right! ..


Anyone else using this.. If you have request caching on and things are going wonky for you please comment. I'll leave this open for a bit see if it catches anyone's eye!.

saltpeanuts commented 7 years ago

All sounds good!

Seems like your non-scientific tests were more scientific than mine, haha. I'll have to look at that again.

Looking forward to playing with the cache primer.

cdowdy commented 7 years ago

ok.. I "fixed" this. Its not really a fix but a hacky work around.

Instead of returning a symfony streamedresponse I'll just output the image to the browser. Which I haven't fully tested the perf impact yet but it'll at least give anyone with request caching an actual image to see/display.