silverstripe / silverstripe-s3

Silverstripe module to store assets in S3 rather than on the local filesystem (SS4/SS5 only)
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

Flysystem v3 #63

Closed obj63mc closed 10 months ago

obj63mc commented 10 months ago

This pull request updates this module so you can use flysytem v3 with silverstripe assets v2.1 (SS v5.1+) - Fixes #62

Biggest changes are using a new cache provider and updating all dependencies. Also setting default cache to be written to the filesystem. Users upgrading will need to double check their configuration after upgrading as the new cache provider is a PSR 6 cache provider.

This would need to be released as a new major version. I have tested this on one of my sites and all appears to be working okay. Major item that may need to be addressed is the file listing in the admin asset manager is extremely slow on first call until things are cached.

obj63mc commented 10 months ago

When researching this more it was actually silverstripe/assets v2.0 beta that switched to flysystem v3. Users running SS5 and this module were getting silverstripe/assets 2.0 alpha installed. Composer is updated so v5.0+ will now use this.

rskuipers commented 10 months ago

@obj63mc I've been following this development for a while now and I've been updating a fork of the obj63mc/silverstripe-google-cloud-storage package which I'd be happy to PR back to you.

But while updating that package I noticed that there were still a huge amount of calls being made on the first request as you mentioned. After doing some debugging I found an error in the jgivoni/flysystem-cache-adapter.

I've made a pull request there, I think this might solve the problem you mentioned with the first load being extremely slow. Cache was basically not being used for the first request.

https://github.com/jgivoni/flysystem-cache-adapter/pull/3/files

obj63mc commented 10 months ago

@rskuipers - that would definitely be an issue but we are also seeing other issues with how SS verifies files exists as they actually stream the file down and then calculate its hash. Until that is actually 'cached' as well that just adds more time. Example is in the CMS Assets Admin - if you have 50 images/files per page it has to literally download all of those, run a hash and then cache it. Definitely not something you would ever want to happen. As to the google-cloud-storage adapter, I no longer have an easy google dev account to test this but I have checkced in updates for v3 at https://github.com/obj63mc/silverstripe-google-cloud-storage/tree/flysystem-v3 if you would like to test those out. Again completely untested but feel free to submit any edits/PRs/issues and we can get that updated as well.

obj63mc commented 10 months ago

After testing the updated cache provider https://github.com/jgivoni/flysystem-cache-adapter with php 8.1+ support which is what SS5 supports this is good to be merged. @GuySartorelli or @emteknetnz - I don't have access to actually release this. Could we have this tagged as v2.0.0 since this now requires SS5.0+

GuySartorelli commented 10 months ago

Tagged as 2.0.0