Added CacheKeyGenerator

[Nyholm]

Introducing CacheKeyGenerator interface. This will make sure one can control how a cache key is generated.

This will fix #30

// Default will use RequestLineAndBodyGenerator
$cachePlugin = new CachePlugin($pool, $streamFactory);

$config = [
  'cache_key_generator' => new MyCustomKeyGenerator(), 
];

$cachePlugin = new CachePlugin($pool, $streamFactory, $config);

[Nyholm]

Im done with this PR now. It is ready for review.

PS: The commit "Support PSR-4" was a typo. I ment to write "Support PHP 5.4"

[Nyholm]

Not sure if I should use null here or if I should instanciate the default generator. Using null will make sure we do not instanceiate unneeded objects.

[sagikazarmark]

I had a few small comments, but looks ok. My fear is that we are putting too much logic here. Do you think the caching logic could be used elsewhere too? If so, should we extract it and move it to message for example?

[sagikazarmark]

I would move this check right after the option resolution. I don't see any benefit in timing here, this will be resolved sooner or later.

[sagikazarmark]

Can we find a shorter name for this?

[Nyholm]

Yes please! Any suggestions?

[sagikazarmark]

Hm, maybe put this under ...\Plugin\Cache\...

[Nyholm]

Thank you for the feedback. I've updated. I renamed the generator to "SimpleGenerator" since it uses an naive approach.

My fear is that we are putting too much logic here. Do you think the caching logic could be used elsewhere too? If so, should we extract it and move it to message for example?

The CachePlugin is pretty fat, yes. One could refactor it to more helper classes and strategies.

Im not sure it make sense to try to reuse the cache logic. Not at this point, because the code it too coupled to this specific use case. But I will not close that door forever.

If I recall correctly we decided to not have cache logic in message because we did not want to have so many external dependencies in php-http/message. But you may be referring to the HTTP specific parts... Again, not sure.

I think we can let this library grow and mature. It is an implementation (and not an abstraction like most of our other libraries) so it is fine to bump it to 2.x and 3.x when we see a need for it.

[dbu]

i like this and think its fine to put in here at least for now. lets improve the cache plugin and if we realize we need the cache key elsewhere we can always push it upstream in a BC way as the cache depends on client-common

[Nyholm]

Thank you for the reviews.

[dbu]

please add documentation for this. i created php-http/documentation#187 so we don't forget.

[Nyholm]

Thank you