'respect_response_cache_directives' is ignored

[nunoplopes]

We have the following code in CachePlugin.php:

    public static function clientCache(CacheItemPoolInterface $pool, StreamFactoryInterface $streamFactory, array $config = [])
    {
        // Allow caching of private requests
        if (\array_key_exists('respect_response_cache_directives', $config)) {
            $config['respect_response_cache_directives'][] = 'no-cache';
            $config['respect_response_cache_directives'][] = 'max-age';
            $config['respect_response_cache_directives'] = array_unique($config['respect_response_cache_directives']);
        } else {
            $config['respect_response_cache_directives'] = ['no-cache', 'max-age'];
        }

        return new self($pool, $streamFactory, $config);
    }

If I pass a config with 'respect_response_cache_directives' => [], it is ignored and replaced with ['no-cache', 'max-age'].
I wanted it to always cache any page regardless of the server directives. It's impossible to do that right now. I'm not sure what's the intention of the code above.

[dbu]

hm, good question indeed. it looks like one can only add additional directives to respect, but not remove no-cache and max-age. this had been added like this with the original clientCache factory method in 1568bea, so it was always like this.

i see that the serverCache factory method does not tamper with that option, so maybe you could use that? (you still need to set the option to empty array, otherwise the OptionsResolver will add a bunch of directives to respect.

[nunoplopes]

The issue is that I'm not using php-http directly but through the php github API (https://github.com/KnpLabs/php-github-api).
But not respecting the respect directive seems wrong to me.

[dbu]

oh i see. you could extend the (Builder)[https://github.com/KnpLabs/php-github-api/blob/9afaf87f99c6c2acde80b6925d1b97a70ece8cb5/lib/Github/HttpClient/Builder.php] and overwrite the addCache method.

while i agree that the current behaviour is unexpected and strange, i am afraid to change it as a bugfix - it would be significant behaviour change

[dbu]

unfortunate timing btw, we just tagged 2.0 2 weeks ago. otherwise we could have declared it one of the important BC breaks of 2.0.

we could add a new factory method that does not force any of the options and deprecate this one, to have a sane upgrade path. and once that is released, we could change the php-github-api to use the new method. do you want to work on that?