Cache-Control header handling

[sagikazarmark]

Q	A
Bug?	no
New Feature?	yes

See php-http/plugins#58

[GrahamCampbell]

👍

[dbu]

we do respect max-age. what is missing is a clearer role concept of how we handle s-maxage / private / no-cache / no-store instructions. as discussed in the other issue, we should probably have two plugins for the roles "single client / browser" and "caching proxy / shared cache". ideas for good names?

[Anahkiasen]

Any progress on this issue? Trying to use the plugin in the context of the php-github-api package but most (all?) Github responses return the private directive which makes the plugin skip caching :/

[dbu]

glad if you can work on it. i think the aproach of having two plugins for the two roles (cache proxy / personal cache) seems cleanest

[acrobat]

I'm also running into this issue while caching github api responses. So I would like to put some work in to this, but how should we fix this?

Deprecate the general CachePlugin and have 2 new cache plugins for "single client" and "shared cache"? Or keep the currect CachePlugin and add the 2 extra caching type classes?

And if I understand correctly the only difference between the 2 new caching types will be the handling of private and no-store?

[dbu]

i think we mainly should add things to the $options to customize the behaviour. a config field for the list of cache-control directives should be respected, instead of the boolean in respect_cache_headers. for BC, we can rewrite respect_cache_headers=false to an empty list. the default value for respect_cache_headers should become the headers we currently look at.

once we have that, we can do a ClientCachePlugin and ServerCachePlugin that set the right defaults for respect_cache_headers, to make code more explicit. but people can also use CachePlugin directly for special situations. in #24 @tuupola is adding support to configure which request methods can be cached.

@php-http/owners what do you think of this approach?

[acrobat]

Sound good to me! Let's see what @php-http/owners think about it, after that I will try to start work on it!

[tuupola]

Would also be nice to have support for request cache headers. Currently they are ignored. For example when client sends request with Cache-Control: no-cache fresh content should be requested from the server.

[sagikazarmark]

The options solution sounds like a good idea to me.

I am not sure about separate plugin classes though. If it's only about special construction, how about just having static constructors? CachePlugin::clientCache and CachePlugin::serverCache

[dbu]

I am not sure about separate plugin classes though. If it's only about special construction, how about just having static constructors? CachePlugin::clientCache and CachePlugin::serverCache oh, factory methods are much better than additional classes. do we whitelist directives? seems more explicit than blacklist to me. we could even check the list and complain about unsupported directives. for request headers, that should be a separate option. but lets do that in a separate PR

[acrobat]

Ok, I will try to start WIP PR with the extra options and factory methods to setup specific caching so we can discuss things there with the code attached!

[acrobat]

I've created #26 for fixing this issue. Please provide some feedback on this first version! Thanks!