drupal_amazons3
drupal_amazons3 copied to clipboard
Cache S3 bucket validation
@deviantintegral Here's the first sketch of Doctrine-caching for the s3 bucket validation calls. You already gave some quick feedback in Slack, so maybe we can paste that here for discussion, and then like you suggested iterate in this PR.
I also opened an issue for this (in case that's helpful for future), with more detailed info: #46
(from a prior chat we had)
a few thoughts.
- there's no static caching the way you have it now, so multiple calls in the same request will still hit memcache. You should probably use a ChainCache along with an ArrayCache and the passed in cache object inside the validateBucketExists method.
- Lets typehint the param as a CacheProvider, because then for testing we don't have to use a "Drupal" cache.
- I agree we should use the cache lifetime setting, but I don't like that we've got coupling now between the client and the streamwrapperconfig object. I'd be tempted to rename it to S3Configuration or something, but we'd need a wrapper class still to preserve BC.
@deviantintegral OK, I moved it all to a similar pattern from Aws\S3\StreamWrapper like we discussed. I put the assumptions we made into comments. Let me know what you think :smile:
@deviantintegral Doesn't look like the failed build is related to this PR
The actual cache code looks fine, though I've only done a static review. It will eventually need unit tests on the new code before we merge it in.