cache-plugin icon indicating copy to clipboard operation
cache-plugin copied to clipboard

'respect_response_cache_directives' is ignored

Open nunoplopes opened this issue 1 year ago • 4 comments

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.

nunoplopes avatar Mar 03 '24 16:03 nunoplopes

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 1568bea5f426955dc815cf67a3d10e99526152ed, 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.

dbu avatar Mar 04 '24 08:03 dbu

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.

nunoplopes avatar Mar 04 '24 09:03 nunoplopes

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 avatar Mar 04 '24 12:03 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?

dbu avatar Mar 04 '24 12:03 dbu