feat: load static assets manually so all other requests rely on wasm_handler #226
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Angelmmiguel
commented
Oct 2, 2023
| @@ -25,7 +25,7 @@ pub async fn handle_assets(req: &HttpRequest) -> Result<NamedFile, Error> { | |||
| // Same as before, but the file is located at ./about.html | |||
| let html_ext_path = root_path.join(format!("public{uri_path}.html")); | |||
|
|
|||
| if file_path.exists() { | |||
| if file_path.exists() && !uri_path.is_empty() && uri_path != "/" { | |||
There was a problem hiding this comment.
We need this new condition to avoid returning the public folder information.
Angelmmiguel
commented
Oct 2, 2023
Comment on lines
-45
to
-53
| if let Some(route) = selected_route { | ||
| // First, check if there's an existing static file. Static assets have more priority | ||
| // than dynamic routes. However, I cannot set the static assets as the first service | ||
| // as it's captures everything. | ||
| if route.is_dynamic() { | ||
| if let Ok(existing_file) = handle_assets(&req).await { | ||
| return existing_file.into_response(&req); | ||
| } | ||
| } |
There was a problem hiding this comment.
Since we're explicitly mounting the static files in certain routes, this is not required anymore.
Angelmmiguel
commented
Oct 2, 2023
Comment on lines
+82
to
+106
| let workers = WORKERS | ||
| .read() | ||
| .expect("error locking worker lock for reading"); | ||
|
|
||
| // Configure the KV store when required | ||
| for route in serve_options.base_routes.routes.iter() { | ||
| let worker = workers | ||
| .get(&route.worker) | ||
| .expect("unexpected missing worker"); | ||
|
|
||
| // Configure KV | ||
| if let Some(namespace) = worker.config.data_kv_namespace() { | ||
| data.kv.create_store(&namespace); | ||
| } | ||
| } | ||
|
|
||
| // Pre-create the KV namespaces | ||
| let data_connectors = Data::new(RwLock::new(data)); | ||
|
|
||
| // Static assets | ||
| let mut static_assets = | ||
| StaticAssets::new(&serve_options.root_path, &serve_options.base_routes.prefix); | ||
| static_assets | ||
| .load() | ||
| .expect("Error loading the static assets"); |
There was a problem hiding this comment.
I extracted all the initialization befure the HttpServer::new call. Actix runs this block on every thread, so we avoid calculating static assets multiple times.
|
@ereslibre don't review this yet, as I need to rethink about this issue. I may close this PR and open a more well-defined issue with a proper set of goals and definition |
|
For now, I'm going to close this PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first PR to complete #175. Before this PR, the default service that captured any non-processed request was the
actix-filesservice (viaFiles). Since we wanted to mount static files into/,Fileswas capturing al/.*routes. For workers,wwswas mounting their routes explicitly.In #175, our intention is the opposite. We want
workers_handlerto manage any unknown route, so we can add new routes when the server is running. My first approach was to find a way to pass a request toworker_handlerfromFilesusing thedefault_handler. TheFilesservice triggers this handler when it doesn't find any matching file. However, the types didn't match and it was an extra step before processing the request.For this reason, I changed the approach and set all the static assets routes manually. Then,
wwssends any unknown route to thewasm_handlerto detect if there's any worker that manages it. In fact, this is a more correct approach as static files have a higher priority than workers (like dynamic).