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

Implement rewrite API #2789

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Implement rewrite API #2789

wants to merge 5 commits into from

Conversation

the10thWiz
Copy link
Collaborator

The goal of this PR is to implement a Rewrite API, following the basic outline presented in #2716.

The goal is to replace the existing Options on FileServer to instead use a rewrite based API. This will make FileServer pluggable, making it possible to implement things like cached compression implementable outside of Rocket.

The basic usage example is as follows:

FileServer::new("static")
   .rewrite(DotFiles)
   .rewrite(NormalizeDirs)
   .rewrite(Index("index.html"))

There are a couple of major limitations to the rewrite trait, specifically the file to be served must exist on the local file system, and the trait is synchronous, so it should not do any IO when rewriting paths. However, I would argue that if you want to move beyond these, you should just implement your own Handler, and fully replace FileServer.

@the10thWiz
Copy link
Collaborator Author

the10thWiz commented May 12, 2024

I've implemented a basic gzip compression rewriter, which provides an API similar to what was discussed in #2716.

FileServer::new("static", Options::None)
   .rewrite(NormalizeDirs)
   .rewrite(Index("index.txt"))
   .rewrite(CachedCompression::new()),

Full example, with implementation: https://github.com/the10thWiz/rocket-caching-layer

This particular implementation generates the compressed version of the file once it's been requested once, and responds with the uncompressed version in the meantime. It's not too hard to imagine different implementations, with different trade-offs. I built this as a proof-of-concept, to show that the new API actually worked for the intended use-case.

I've also marked most of the Options as deprecated, and implemented a quick check to generate an initial set of rewrites based on the Options provided.

@the10thWiz the10thWiz marked this pull request as ready for review May 12, 2024 04:49
@SergioBenitez
Copy link
Member

I'm about to review this. Note that I tried to implement this API myself some time ago and ran into a bunch of issues. Here's my attempt, in case it helps with anything: https://paste.rs/wEcVu.rs

Hopefully you've manage to overcome the limitations I found!

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

Successfully merging this pull request may close these issues.

None yet

2 participants