Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vary #4

Open
sagikazarmark opened this issue May 4, 2016 · 7 comments
Open

Vary #4

sagikazarmark opened this issue May 4, 2016 · 7 comments

Comments

@sagikazarmark
Copy link
Member

Q A
Bug? no
New Feature? yes

See php-http/plugins#59

@Nyholm
Copy link
Member

Nyholm commented Aug 2, 2016

@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
Copy link
Contributor

dbu commented Aug 2, 2016

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
Copy link
Member

Nyholm commented Aug 2, 2016

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

@joelwurtz
Copy link
Member

joelwurtz commented Aug 4, 2016

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
Copy link
Contributor

dbu commented Aug 4, 2016

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:

  • it is written around the symfony Request and not PSR-7. we could write an adapter to solve this but meh we would convert request and response between psr and symfony.
  • it has a hard built-in file system storage instead of using the PSR cache api. afaik symfony recently got a caching component and people work on using that within symfony itself, so there might be progress.

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

@joelwurtz
Copy link
Member

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
Copy link
Contributor

dbu commented Aug 4, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants