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

Add support for caching headers primarily ETag/LastModified/If-Non-Match #136

Closed
prabirshrestha opened this issue Sep 5, 2022 · 10 comments

Comments

@prabirshrestha
Copy link
Contributor

I would like to set appropriate caching headers for any responses include html and apis besides file. This improves performance on slow network when fetching it again.

I would like to have similar feature to trillium's caching-headers.

(In case you are wondering why I'm filing issue, I'm trying to port by blog from trillium to salvo https://github.com/prabirshrestha/rblog with the main reason being trillium not support first class error handling compared to salvo)

chrislearn added a commit that referenced this issue Sep 7, 2022
@chrislearn
Copy link
Member

Done

@prabirshrestha
Copy link
Contributor Author

I can't seem to use this feature. Probably need to add the feature here?

cargo add salvo --features=caching-headers
    Updating crates.io index
      Adding salvo v0.33.0 to dependencies.
             Features:
             + caching-headers
             + cookie
             + test
             - acme
             - affix
             - anyhow
             - basic-auth
             - compression
             - cors
             - csrf
             - extra
             - full
             - jwt-auth
             - logging
             - native-tls
             - openssl
             - proxy
             - rustls
             - salvo_extra
             - serve-static
             - size-limiter
             - sse
             - timeout
             - unix
             - ws
error: unrecognized features: ["caching-headers"]

chrislearn added a commit that referenced this issue Sep 8, 2022
@chrislearn
Copy link
Member

I forgot add it to Cargo.tom. Fixed.

@prabirshrestha
Copy link
Contributor Author

It is able to compile but don't think it is working as expected.

  let router = Router::new()
      .hoop(extra::caching_headers::CachingHeaders::default())
      .hoop(extra::compression::Compression::default())
      .get(hello_world);

I was expecting curl -vv http://localhost:8080 to return ETag header in the response.

Screen Shot 2022-09-08 at 9 13 49 PM

@chrislearn
Copy link
Member

I fixed it, please check it again.

@prabirshrestha
Copy link
Contributor Author

Did you push the changes? I see version 0.33.1 as the latest commit.

@chrislearn
Copy link
Member

Please check again, version 0.33.2.

@prabirshrestha
Copy link
Contributor Author

If it is uncompressed response it works. curl -vv http://localhost:8080.

As soon as I compress it doesn't work. curl -vv --compressed http://localhost:8080. Since most of the content html/css/js/json api are compressed it should support etags when content is compressed.

@chrislearn
Copy link
Member

Fixed

@prabirshrestha
Copy link
Contributor Author

Everything now works as expected. Thanks for fixing the bugs along the way!

Feel free to release a new version so I can consume it directly from crates.io

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

No branches or pull requests

2 participants