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

Cache control of static resources #831

Closed
veloman-yunkan opened this issue Oct 7, 2022 · 4 comments · Fixed by #833
Closed

Cache control of static resources #831

veloman-yunkan opened this issue Oct 7, 2022 · 4 comments · Fixed by #833
Assignees
Milestone

Comments

@veloman-yunkan
Copy link
Collaborator

This is a sub-ticket of #650

#712 has introduced support for versioning static resources, however the HTTP caching side of their handling has not been addressed.

Currently a build of kiwix-serve containing static resource /skin/RESOURCE with the cache id value CACHEID is supposed only to receive and handle requests of the form /skin/RESOURCE?cacheid=CACHEID. #650 proposes responding to such requests with Cache-Control: max-age=31536000, immutable. That's easy.

Questions:

  1. What should be the Cache-Control response to a request without any cache-id (/skin/RESOURCE)?
    a. Iframe-based content viewer #716 has introduced a special user-facing static resource served at /viewer which is going to be accessed without any cache id. What should be the caching policy for that one (especially in interaction with [kiwix-serve] make the topbar customizable. kiwix-tools#17)?
  2. What should be the response to a request with a wrong cache-id (/skin/RESOURCE?cacheid=WRONGCACHEID)?
@veloman-yunkan veloman-yunkan self-assigned this Oct 7, 2022
@kelson42 kelson42 added this to the 12.1.0 milestone Oct 7, 2022
@kelson42
Copy link
Collaborator

kelson42 commented Oct 7, 2022

@veloman-yunkan In both cases a temporary redirection ti the URL with proper cacheid? What aboit head requests?

@mgautierfr
Copy link
Member

In both cases a temporary redirection ti the URL with proper cacheid?

I don't think so. At least definitively not for case # 2.

The purpose of the cacheid is to avoid having the client using resources with different versions in the same time.
If we have a request with a "wrong" cache-id, it means that browser is using a out of date cached version of the front and so, do request for resources out of date with what we can serve.
We must return some kind of error to let the user know that it is out of sync and that it must do a full refresh.

For case # 1, it means that user does not ask for a specific version. Most of the time it means user does a specific request and are not using the ui (as all our ui must used cachedid).
I would say that by default we must return the content we have but disabling cache

There is the specific use case of the /viewer which it is somehow the entry point of the ui. And it is normal that access are done without cache id. We could do a redirect to a "cached-id url" but I afraid it will pollute the url displayed to user with some kind of
library.kiwix.org/viewer?cacheid=CACHEDID#path/to/content/article.
I think it is better to simply return the content of the viewer with a relatively small cache time in Cache-Control (1h ?)
Redoing a HEAD request every hour to check viewer version will not hurt us and if somehow the version has changed user will see it pretty soon.

@kelson42
Copy link
Collaborator

kelson42 commented Oct 7, 2022

Do we have legit scenarios where user will be out of sync? One of the primary goals is that the user never has to refresh manually.

@kelson42
Copy link
Collaborator

kelson42 commented Oct 7, 2022

@mgautierfr Analysis seems pretty accurate to me. My first comment might not be that good. But we should do our best to avoid any legit scenario where user has to refresh manually. Maybe viewer should be cached and refresh if necessary based on an etag (we don't request it that ofyen anyway, each time we open a new tab I guess)? The only problematic scenario would be if API change an tab survives... then the user woyld have buggy behaviour as long as he keeps using the same tab (maybe we could have a background job checking and refteshing if necessary?)

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

Successfully merging a pull request may close this issue.

3 participants