Implement first idea of the cache provider

[Baachi]

Hey guys,

PSR6 has been released weeks ago and so we have a standard for caching. The feature has been wished thousand times so i started to work on it.
Currently this is only a proof of concept and i will finished it if we all fine with the idea.

My intention is currently: We provide multple strategies and the user can decide how he wants to cache the response.
Currently i wrote the stale-if-error strategy and the expire strategy, because i think these are the most common behaviours.

Please provide feedback 😄

[willdurand]

👍

[willdurand]

Also, Symfony 3.1 will have a Cache component.

[Baachi]

Oh didn't know that, but wouldn't it be better to build against the psr/cache interface?

[drupol]

I've implemented it in the Drupal module using custom code, without strategy. Code here: https://github.com/drupol/geocoder/blob/8.x-2.x/src/Plugin/Geocoder/Provider.php#L72

[toin0u]

@Baachi Symfony added the PSR-6 implementation symfony/symfony#17408 :)

[Baachi]

@toin0u Cool didn't know that 👍

[joelwurtz]

See #487 if it's only for HTTP request we (php-http) already provide a plugin for caching response with PSR6, this will maybe allow to reduce the maintenance on your side

[Baachi]

@joelwurtz I know that httplug have a caching system but it uses the cache-control header and many providers doesn't set cache control header.
I would like to have a caching strategy which cover the following use cases:

• The user hit the api limit, so load it from the cache so he have at least a result
• The user wants to reduce the api limit, so use a cache

If this is possible through httplug, let me know.

[joelwurtz]

The user wants to reduce the api limit, so use a cache

The CachePlugin as an option to respect / ignore cache headers :respect_cache_headers, si if the request is within an acceptable method (GET or HEAD) and an acceptable status code (200, 203, 300, 301, 302, 404, 410) it is cached, even if the cache-control was set to no-cache.

The user hit the api limit, so load it from the cache so he have at least a result

In fact the CachePlugin has a protected method to determine if the response is cacheable or not (see https://github.com/php-http/plugins/blob/master/src/CachePlugin.php#L104). So for this use case you can extend the cache plugin and implementing the rule for api limit.

You also give me an idea about introducing a StaleCachePlugin, like caching response in another cache with a longer ttl, and if a following request / response is in failure, it reuse the cached one.

[Baachi]

@joelwurtz This sounds indeed interesting and would also cover this use case.

In fact the CachePlugin has a protected method to determine if the response is cacheable or not (see https://github.com/php-http/plugins/blob/master/src/CachePlugin.php#L104). So for this use case you can extend the cache plugin and implementing the rule the api limit.

Yes this is right, but how would you detect if there is an error in this response? Geocoder already know this already and throw an exception if the provider hit the api limit or there is an other error.

[Baachi]

@geocoder-php/geocoder ready for review 👍

[toin0u]

@Baachi Good job! It looks good 👍 Just few missing phpdocs :)

[willdurand]

👍

[willdurand]

@Baachi any update on this PR?

[Baachi]

Will try to finish up this PR this week.

[Nyholm]

FYI: One could use the CachePlugin to HTTPlug.

[Baachi]

@willdurand do you still want this PR? If yes i will add the missing docs

[Nyholm]

Do we need this PR?
See this cookbook: https://github.com/geocoder-php/Geocoder/blob/master/docs/cookbook/cache.md

Doesn't that solve the cache problem for us?

[Baachi]

@Nyholm this is the question, the only thing which isn't currently not covered is the stale-if-error strategy. But of course i can also open a PR to HTTPLug to cover this case.

[Nyholm]

I would vote for not merging this and add stale-if-error on the https://github.com/php-http/cache-plugin

[Baachi]

@Nyholm Should i open an issue on the php-http/cache-plugin repo to discuss the implementation or are you okay with this implementation?

[Nyholm]

I think this implementation looks good. Since it is not too much code, I suggest to open a quick PR and we'll discuss it there. Do not spend too much time (or none) at the tests until you got some feedback.
There are some people watching that repo that are really skilled at HTTP cache. I guess they will have some input as well.

[atymic]

@Nyholm

Is this PR still needed?
It seems we have a CacheProvider in the package, as well as instructions on how to use the httplug cache?

[Baachi]

I think this PR is not needed anymore :)
Sadly i didn't have time, to finish it 😢

[atymic]

Thanks @Baachi 😄