Cache S3 bucket validation

[scottrigby]

@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

[deviantintegral]

(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.

[scottrigby]

@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 😄

[scottrigby]

@deviantintegral Doesn't look like the failed build is related to this PR

[deviantintegral]

This is the line that's cause the test to fail.

The way we stub procedural calls like variable_get() is to drop them into a trait that can then be swapped in a sub class. For example, StreamWrapperConfiguration uses Drupal\amazons3\DrupalAdapter\Bootstrap to call variable get, while the StreamWrapperConfiguration stub under the tests directory uses Drupal\amazons3Test\DrupalAdapter\Bootstrap.

In other words, this comes down to this code not injecting the StreamWrapperConfiguration class so we can swap it during tests.

I think there's two followups here to fix this:

1. Let's make a class property with the string of the full class to call here. Then we can add a setter around it to override it in tests, and have the default be \Drupal\amazons3\StreamWrapperConfiguration.
2. This adds some strange coupling conceptually between the S3 client and the stream wrapper configuration class. I was probably a little too narrow in calling the class StreamWrapperConfiguration. Let's copy it to AmazonS3Configuration? Or just Configuration since it's namespaced? Then we mark the StreamWrapperConfiguration class as @deprecated and all of it's methods just call out to the Configuration class (preserving backwards compatibility). In fact... I think this refactor could be done as an entirely separate PR. We get that one in first before finishing this one off.

Sound good?

[deviantintegral]

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.