search

bolt / thumbs

Thumbnail handler for Bolt Images
12 stars 16 forks source link

Mount point in path #31

Closed CarsonF closed 8 years ago

CarsonF commented 8 years ago

We need to be able to include mount point in path. Currently we are guessing by checking each filesystem to see if the file exists 👎 . It needs to be optional for BC, which puts us in a tight spot.

Ideally would do this: /thumbs/5x5c/theme/foo/bar.jpg But when it is not included, /thumbs/5x5c/foo/bar.jpg: mount point would be foo and path would be bar.jpg.

We could do one of these:

/thumbs/theme/5x5c/foo/bar.jpg <= works because 5x5c separates the regex
/thumbs/5x5c/foo/bar.jpg?mountPoint=theme <= Bleh :(

We could also do another controller prefix. That would create a bit more work, but it's doable. 

/thumb/5x5c/theme/foo/bar.jpg
/thumbnail/5x5c/theme/foo/bar.jpg

Any other thoughts?

GwendolenLynch commented 8 years ago

We could also do another controller prefix.

More work, but wouldn't that be a clearer way forward?

CarsonF commented 8 years ago

More work, but wouldn't that be a clearer way forward?

Probably. But it means duplicating the controller and we have a filesystem problem too. If saving thumbnails to disk is enabled then there are two folders...I guess that's not terrible though.

Definitely pays off in the long run not having to guess which filesystem to use.

GwendolenLynch commented 8 years ago

Could we have the old controller extend the new controller and call parent functions with a corrected path, or am I miss understanding the problem? (It's Monday after all)

CarsonF commented 8 years ago

So creating a new controller would present a bunch of work, because of the caching aspect (thumbnails are saved to filesystem).

I think we could handle the change in the same controller there would just be some edge cases. Say you had: files://files/foo/bar.jpg that being a files folder instead the files folder/filesystem. It would previously generate a route like this: /thumbs/5x5c/files/foo/bar.jpg. If that was used on the controller (with my change), it would be parsed as files://foo/bar.jpg, missing the inner files folder. As long as all cache is invalidated, including cache gateways, then the route would be regenerated like this: /thumbs/5x5c/files/files/foo/bar.jpg which would be parsed correctly.

I think this edge case is small enough that it probably won't be a problem. As long as cache is invalidated, so the thumb routes are regenerated correctly.

GwendolenLynch commented 8 years ago

Works for me.

CarsonF commented 8 years ago

K I'll work on PR at some point soon.

CarsonF commented 8 years ago

Ok I lied I can't do it in same controller without breaking BC.

Everyone ok if I do a major version for this change?

GwendolenLynch commented 8 years ago

Modified approach approved at dev-meeting.

@CarsonF PR away :+1: