Refactor path params and optimize JSON handling #12
Conversation
Replaces HashMap usage with PathParams for request path parameters throughout core and middleware modules. Switches JSON parsing and serialization to use simd-json for improved performance and adds buffer pre-allocation for responses. Implements CORS middleware logic in rustapi-extras, handling preflight and actual requests with correct headers. Updates tests to use PathParams.
There was a problem hiding this comment.
Pull request overview
This PR aims to refactor path parameter handling from HashMap to a new PathParams type and switch to simd-json for improved JSON performance. It also implements CORS middleware logic in rustapi-extras. However, the PR has critical compilation errors that prevent it from being merged.
Changes:
- Attempts to replace HashMap with PathParams for request path parameters in tests
- Attempts to switch JSON parsing/serialization to use a new
jsonmodule (not included in PR) - Implements CORS middleware with preflight and actual request handling
- Adds two image files to .gitignore
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rustapi-extras/src/cors/mod.rs | Implements MiddlewareLayer trait for CorsLayer with preflight and actual request handling |
| crates/rustapi-core/src/middleware/tracing_layer.rs | Updates test to use non-existent PathParams type |
| crates/rustapi-core/src/middleware/request_id.rs | Updates test to use non-existent PathParams type |
| crates/rustapi-core/src/middleware/layer.rs | Updates test to use non-existent PathParams type |
| crates/rustapi-core/src/middleware/body_limit.rs | Updates test to use non-existent PathParams type |
| crates/rustapi-core/src/extract.rs | Updates tests to use non-existent PathParams type and attempts to use non-existent json module |
| crates/rustapi-core/src/app.rs | Updates test to use non-existent PathParams type and attempts to use non-existent trace_info! macro |
| .gitignore | Adds two image files to ignore list |
Comments suppressed due to low confidence (1)
crates/rustapi-core/src/app.rs:285
- The macro
crate::trace_info!doesn't exist in the codebase. This will cause a compilation error. You should either usetracing::info!directly with the conditional compilation guard, or define thetrace_info!macro.
crate::trace_info!(
paths = path_count,
routes = route_count,
"Auto-registered routes"
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/rustapi-core/src/extract.rs
Outdated
| Bytes::new(), | ||
| Arc::new(Extensions::new()), | ||
| HashMap::new(), | ||
| PathParams::new(), |
There was a problem hiding this comment.
The type PathParams doesn't exist. Based on the code in request.rs, path parameters are still stored as HashMap<String, String>, not PathParams. This will cause a compilation error.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
crates/rustapi-core/src/extract.rs
Outdated
| Bytes::new(), | ||
| Arc::new(Extensions::new()), | ||
| HashMap::new(), | ||
| PathParams::new(), |
There was a problem hiding this comment.
The type PathParams doesn't exist. This will cause a compilation error.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| Bytes::new(), | ||
| Arc::new(Extensions::new()), | ||
| HashMap::new(), | ||
| PathParams::new(), |
There was a problem hiding this comment.
The type PathParams doesn't exist. This will cause a compilation error.
Replaces usage of the PathParams struct with std::collections::HashMap throughout the codebase, including tests. Also removes dependency on the custom json module in favor of serde_json for JSON serialization and deserialization. Updates logging to use tracing directly instead of crate::trace_info! macro.
Reformatted the CorsLayer middleware logic to improve code readability by expanding chained method calls and aligning insertions for response headers. No functional changes were made. Minor whitespace cleanup in extract.rs and app.rs.
Introduces a new example 'cors-test' to demonstrate CORS, rate limiting, and middleware usage with rustapi-rs. Updates workspace members in Cargo.toml and bumps rustapi-related crate versions to 0.1.8.
Replaces HashMap usage with PathParams for request path parameters throughout core and middleware modules. Switches JSON parsing and serialization to use simd-json for improved performance and adds buffer pre-allocation for responses. Implements CORS middleware logic in rustapi-extras, handling preflight and actual requests with correct headers. Updates tests to use PathParams.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of Change
Checklist
Related Issues
Fixes #11
Testing
Please describe the tests that you ran to verify your changes: