php-http / cache-plugin

PSR-6 Cache plugin for HTTPlug
http://docs.php-http.org/en/latest/plugins/cache.html
MIT License
250 stars 18 forks source link

Vary #4

Open sagikazarmark opened 8 years ago

sagikazarmark commented 8 years ago
Q A
Bug? no
New Feature? yes

See php-http/plugins#59

Nyholm commented 8 years ago

@dbu wrote:

the CachePlugin currently does not respect the Vary header. vary by its nature can not be part of the cache key (we don't know the vary from the request, only from the response). but we should check the response we find for a Vary header and compare the headers from the original request with the ones from the current request.

in many scenarios, we will never vary (e.g. different accept-encodings are unlikely) but e.g. when sending requests on behalf of a logged in user, we might do vary.

I agree that we should be more sophisticated when creating the cache key.

Since we cant put the data from the vary header in the cache, it would mean that we have to make a request to the server an all request.. so vary has no effect.

A possible solution is to have a storage/repository with cache keys, vary headers and responses. So at each request we look in the storage. If cached vary header exists we validates those headers on the request and return a response if we got one.

dbu commented 8 years ago

i think what we need is a cache that would give us all variants we currently have. we would look at each candidate and check our headers for the vary headers and if they match. if we find no match, we send the request and add that variant to the cache. we could probably make the assumption that all variants have to have the same vary headers - then we could move the definition of what this varies on to the cache entry instead of looking at each response. would have to check the rfc about the vary header.

the thing is that with vary, requests will lead to the same cache entry regardless of vary, because at that point we don't know what to vary on. the cache entry then must provide several possible responses, distinguished by the headers as defined in vary.

Nyholm commented 8 years ago

Yeah, that was pretty much was I was trying to say.

joelwurtz commented 8 years ago

So this will be the following ? :

ResponseCache -> Cache Response without it's body BodyCache -> Cache Body of response

ResponseCache key = targetUrl of the request (there can be multiple responses for a same targetUrl, so we store an array of Response) BodyCache key = vary hashing (we took the headers to vary on from the response, their values from the request, create a cache key and retrieve the body from the cache if exists, or make the actual request if not)

dbu commented 8 years ago

we could port (or reuse?) the store that symfony HttpCache uses: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpCache/Store.php#L140-L165

symfony HttpCache\Store has two flaws for us:

i fear it will be easier to just port the relevant logic from symfony to this plugin.

joelwurtz commented 8 years ago

For converting psr7 we can also use their component also ? and make a PR in symfony for the storage, but for me we could maybe just provide two plugins, one withtout symfony dependency with a small feature set, and another one using directly symfony.

dbu commented 8 years ago

let whoever wants to work on this check what is easier.

i am afraid that using the symfony httpkernel component is a lot of additional code for a class that in the end is not ideal. in caching, speed is important and converting between psr 7 and symfony requests for cache lookups feels really poor to me. so much as i am for code reuse, this might be a case where reuse is not a good solution.