Browse version API should be faster when cached

kumar303 commented 4 years ago

Describe the problem and steps to reproduce it:

Open the Network panel
Browse a version in Code Manager like https://code.addons-dev.allizom.org/en-US/browse/616959/versions/1688394/
Refresh the page

What happened?

When refreshing the page, the browser loads a cached version of the response (/api/v5/reviewers/addon/616959/versions/1688394/) as indicated in the Network panel. However, the cached response takes the same amount of time as the non-cached one.

What did you expect to happen?

The cached response should be faster :rocket:

Anything else we should know?

This is probably similar to https://github.com/mozilla/addons/issues/7100 where the server still calculates the response even when cached.

Example of when the browser makes a cached request:

curl 'https://addons-dev.allizom.org/api/v5/reviewers/addon/616959/versions/1688394/?lang=en-US' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:70.0) Gecko/20100101 Firefox/70.0' -H 'Accept: */*' -H 'Accept-Language: en-US,en;q=0.5' --compressed -H 'Authorization: Bearer <redacted>' -H 'Origin: https://code.addons-dev.allizom.org' -H 'Connection: keep-alive' -H 'If-None-Match: W/"89659eb47790ae60aedfa2f2b5f5df09"'

┆Issue is synchronized with this Jira Task

diox commented 4 years ago

While I am not sure we want to open the can of worms that is server-side caching yet...There might be endpoints we can serve with a Cache-Control header, to let clients cache the response for themselves ? The browse endpoint seems like a good candidate, besides the add-on properties I don't think anything can change after it has been loaded.

That being said, we could also make If-None-Match/If-Modified-Since requests faster by looking at the modified date of the version/file object, or something from the git backend.

kumar303 commented 4 years ago

Yeah, in this case the browser has a cached response and is sending a valid etag in the request. The server agrees and responds with a 304. Is the problem that the API has to create a real response before it can figure out if the etag is valid or not?

diox commented 4 years ago

Yeah, the default implementation is to generate ETag based on the response (it's a simple hash of the response body), and that's what we use for this API. We could be smarter about this. Returning a Cache-Control would also sidestep this issue as the browser wouldn't send a request for the duration of the max-age.

KevinMind commented 4 months ago

Old Jira Ticket: https://mozilla-hub.atlassian.net/browse/ADDSRV-24

mozilla / addons