SERIOUS BUG IN NEW VERSION

[GrahamCampbell]

Requests are being cached incorrectly, so that API calls to get a users repo are now returning cached results from the wrong user. I am rolling back to the previous version immediately.

[acrobat]

I think the generated cache key is to general. See https://github.com/php-http/cache-plugin/blob/master/src/CachePlugin.php#L290

If the request parameters are included the key will be unique enough I think

Thoughts on this @Nyholm?

[GrahamCampbell]

This is only in the very latest update. No such bug occurred 7 days ago.

[acrobat]

That's because the caching of requests was broken in previous versions. All requests with the private or no-cache directive were skipped by the cache plugin. So no github api request could be cached

You can fix this for now by passing this option to the cache plugin. Example from the docs:

use Cache\Adapter\Redis\RedisCachePool;

$client = new \Redis();
$client->connect('127.0.0.1', 6379);
// Create a PSR6 cache pool
$pool = new RedisCachePool($client);

$client = new \Github\Client();
$option = [ 'respect_response_cache_directives' => ['no-cache', 'private', 'max-age', 'no-store']];
$client->addCache($pool, $options);

[acrobat]

@GrahamCampbell Can you provide some example call which fails when using the cache? So I can check how to fix this

[Nyholm]

Thank you for reporting this. I will make a PR.

[Nyholm]

I've added #558. Please review it.

We are just waiting for a new tag on the CachePlugin.

[GrahamCampbell]

Also, the current version of this package is throwing a deprecation error, even know I'm not using a deprecated feature?

[GrahamCampbell]

https://github.com/GrahamCampbell/Laravel-GitHub/blob/master/src/GitHubFactory.php#L86

[Nyholm]

Also, the current version of this package is throwing a deprecation error, even know I'm not using a deprecated feature?

From where?

[GrahamCampbell]

It's because you set a deprecated default: https://github.com/php-http/cache-plugin/blob/v1.3.0/src/CachePlugin.php#L336.

[GrahamCampbell]

Surely this still won't fix everything. Shouldn't the caching have only been etag based, why on earth would we provide a correct etag, and then fetch the wrong cache response?