search

inveniosoftware / invenio-records-rest

Invenio records REST API module.
https://invenio-records-rest.readthedocs.io
MIT License
4 stars 62 forks source link

REVIEW NEEDED [WIP] global: add vary header to the response #273

Closed kpsherva closed 5 months ago

kpsherva commented 5 years ago

FOR THE REVIEWERS: this is a proposal of the solution, it will be applied not only for the get method but please first share your thoughts

( addresses #250)

Update: The main issue seemed to be that the vary header had no effect if the etag was unchanged so the new WIP version has the etag include not only the record.revision_id but also the mimetype. It's still unclear which effect the Vary header has but without it sometimes it doesn't recognize if a change is made in the Accept header so it seems to be needed.

Requires https://github.com/inveniosoftware/invenio-rest/pull/100

PAST NOTES ON THE ISSUE:

THIS IS not working fully - below my investigation How to test:

  1. Create clean instance
  2. Install latest invenio 3.2.0a5
  3. install invenio-records-rest from this PR 3.1. on your clean instance create custom serializer of the record f.e. application/x-custom mimetype - make sure it returns also this content type as a response example: https://github.com/kprzerwa/test-headers-instance/blob/master/my_site/records/config.py

You can use firefox for debugging - it has neat way of editing headers in the dev tools

  1. post a new record
  2. go to /records/1
  3. response should have Vary: Content-Type, Accept header - check this PR
  4. Hit again the same endpoint - the response should be cached. But it will not have the Vary header anymore! <--- I think this is where the problem is located
  5. change the Accept header to your custom application/x-custom in the request and request again the endpoint. You will get the same json response, cached, while you should get 200 status code response with x-custom content type. What is probably the problem here: The cached response is not adding the Vary header so the browser does not know that it should check the Accept value every time, and relies only on the combination of ETag (which is version of the record) passed to If-None-Match header and If-Modified-Since header which won't change in this case.
harunurhan commented 3 years ago

As far as I have seen, Vary doesn't seem to fix anything. Browsers would still send If-Match: <Etag> for requests with Accept: <different content-type than the request where we got the Etag in If-Match>, then server would return 304and cause the same bug.