Add TypedPath and ApiError derive macros with typed routing #38
Conversation
Introduces the TypedPath trait and derive macro for type-safe, declarative route definitions and URL generation. Adds a Typed extractor for ergonomic path parameter extraction. Implements the ApiError derive macro for declarative error handling. Refactors test client into rustapi-testing crate, updates re-exports, and adds integration and derive macro tests. Updates documentation and examples to showcase new features.
There was a problem hiding this comment.
Pull request overview
This pull request introduces typed routing and declarative error handling to the RustAPI framework through new TypedPath and ApiError derive macros. It also refactors the test client infrastructure by moving it to a dedicated crate, exposes several internal APIs, and adds examples demonstrating the new features.
Changes:
- Added
TypedPathtrait and derive macro for type-safe route definitions with URL generation - Added
ApiErrorderive macro for ergonomic error handling with declarative status codes and messages - Refactored
TestClientfromrustapi-coreto standalonerustapi-testingcrate with new dependency structure
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rustapi-core/src/typed_path.rs | Defines TypedPath trait for typed routing |
| crates/rustapi-core/src/extract.rs | Adds Typed extractor for typed path parameters |
| crates/rustapi-core/src/router.rs | Adds Router::typed method and exposes internal APIs |
| crates/rustapi-core/src/app.rs | Adds RustApi::typed method |
| crates/rustapi-core/src/request.rs | Makes BodyVariant and Request::new public |
| crates/rustapi-core/src/lib.rs | Exports Typed, TypedPath, and other newly public types |
| crates/rustapi-macros/src/lib.rs | Implements TypedPath and ApiError derive macros |
| crates/rustapi-testing/src/client.rs | Refactors TestClient to depend on rustapi-core, removes tests |
| crates/rustapi-testing/src/lib.rs | Exports TestClient types publicly |
| crates/rustapi-testing/Cargo.toml | Adds rustapi-core dependency |
| crates/rustapi-rs/src/lib.rs | Exports TypedPath derive but missing ApiError derive |
| crates/rustapi-rs/tests/typed_path_derive.rs | Tests for TypedPath derive macro |
| crates/rustapi-rs/tests/api_error_derive.rs | Tests for ApiError derive macro |
| crates/rustapi-rs/examples/typed_path_poc.rs | Example demonstrating typed routing |
| tests/integration/main.rs | Replaces placeholder test with basic integration test |
| README.md | Documents new ApiError derive and TestClient features |
| docs/cookbook/theme/custom.css | Fixes CSS syntax error (closing brace) |
| crates/rustapi-extras/src/cors/mod.rs | Reformats long conditional expression |
| tasks.md | Removes engineering roadmap file |
Comments suppressed due to low confidence (1)
crates/rustapi-testing/src/client.rs:439
- All tests for TestClient have been removed from this file (314 lines of tests deleted). While there is a new integration test in tests/integration/main.rs, it only covers basic GET and POST scenarios. The removed tests included important edge cases like method not allowed (405), custom headers, raw body handling, and property-based tests. These comprehensive tests should be preserved, either by keeping them in this file or moving them to a dedicated test file in crates/rustapi-testing/tests/.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self | ||
| } | ||
|
|
||
| /// Add a typed route |
There was a problem hiding this comment.
The typed method lacks documentation explaining its purpose and providing a usage example. Users may not understand how it differs from the regular route method or how to use it with the TypedPath trait. Add a doc comment with an example showing how to use it with a TypedPath-derived struct.
| /// Add a typed route | |
| /// Add a typed route | |
| /// | |
| /// This is a convenience wrapper around [`RustApi::route`] that uses a path | |
| /// type implementing [`crate::typed_path::TypedPath`] instead of a raw `&str`. | |
| /// The route path is taken from the associated constant [`crate::typed_path::TypedPath::PATH`] | |
| /// on the supplied type `P`. | |
| /// | |
| /// This allows you to keep the path definition and its extracted parameters in one | |
| /// strongly-typed struct, which can then be used as an argument to your handler. | |
| /// | |
| /// # Example | |
| /// | |
| /// ```rust,ignore | |
| /// use rustapi_rs::prelude::*; | |
| /// use rustapi_rs::typed_path::TypedPath; | |
| /// | |
| /// #[derive(TypedPath)] | |
| /// #[typed_path("/users/{id}")] | |
| /// struct UserPath { | |
| /// id: i32, | |
| /// } | |
| /// | |
| /// async fn get_user(path: UserPath) -> impl IntoResponse { | |
| /// // `path.id` is automatically parsed from the URL segment `{id}` | |
| /// // ... | |
| /// } | |
| /// | |
| /// #[tokio::main] | |
| /// async fn main() -> Result<()> { | |
| /// RustApi::new() | |
| /// // Equivalent to: .route("/users/{id}", get(get_user)) | |
| /// .typed::<UserPath>(get(get_user)) | |
| /// .run("127.0.0.1:8080") | |
| /// .await | |
| /// } | |
| /// ``` |
| // Generate to_uri implementation | ||
| // We need to parse the path and replace {param} with self.param | ||
| let mut format_string = String::new(); | ||
| let mut format_args = Vec::new(); | ||
|
|
||
| let mut chars = path.chars().peekable(); | ||
| while let Some(ch) = chars.next() { | ||
| if ch == '{' { | ||
| let mut param_name = String::new(); | ||
| while let Some(&c) = chars.peek() { | ||
| if c == '}' { | ||
| chars.next(); // Consume '}' | ||
| break; | ||
| } | ||
| param_name.push(chars.next().unwrap()); | ||
| } | ||
|
|
||
| if param_name.is_empty() { | ||
| return syn::Error::new_spanned( | ||
| &input, | ||
| "Empty path parameter not allowed in typed_path", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
|
|
||
| format_string.push_str("{}"); | ||
| let ident = syn::Ident::new(¶m_name, proc_macro2::Span::call_site()); | ||
| format_args.push(quote! { self.#ident }); | ||
| } else { | ||
| format_string.push(ch); | ||
| } | ||
| } |
There was a problem hiding this comment.
The TypedPath derive macro does not validate that all path parameters have corresponding struct fields. If a path like "/users/{id}/posts/{post_id}" is used with a struct that only has an id field, the generated to_uri implementation will fail at compile time with an unclear error about a missing field. Consider extracting the path parameters from the path string and validating that each parameter name corresponds to a field in the struct, providing a clear error message during macro expansion if there's a mismatch.
|
|
||
| let _app = RustApi::new() | ||
| // Type-safe registration! | ||
| // The path string is derived from UserParam::PATH |
There was a problem hiding this comment.
The comment references "UserParam::PATH" but the actual type name is "UserPath". This inconsistency could confuse readers of the example.
| // The path string is derived from UserParam::PATH | |
| // The path string is derived from UserPath::PATH |
| /// Trait for defining typed paths | ||
| /// | ||
| /// This trait allows structs to define their own path pattern and URL generation logic. | ||
| /// It is usually implemented via `#[derive(TypedPath)]`. | ||
| pub trait TypedPath: Serialize + DeserializeOwned + Send + Sync + 'static { | ||
| /// The URL path pattern (e.g., "/users/{id}") | ||
| const PATH: &'static str; | ||
|
|
||
| /// Convert the struct fields to a path string | ||
| fn to_uri(&self) -> String; | ||
| } |
There was a problem hiding this comment.
The documentation for the TypedPath trait lacks an example showing how to use it with the derive macro and the Typed extractor. While it mentions the trait is "usually implemented via #[derive(TypedPath)]", including a complete example would make this more helpful. Consider adding an example similar to those in the derive macros or test files.
| #[tokio::test] | ||
| async fn test_basic_api_flow() { | ||
| // Define a handler | ||
| async fn hello() -> &'static str { | ||
| "Hello, World!" | ||
| } | ||
|
|
||
| async fn echo(body: String) -> String { | ||
| body | ||
| } | ||
|
|
||
| // Setup app | ||
| let app = RustApi::new() | ||
| .route("/hello", get(hello)) | ||
| .route("/echo", post(echo)); | ||
|
|
||
| // Use TestClient | ||
| let client = TestClient::new(app); | ||
|
|
||
| // Test GET | ||
| client.get("/hello") | ||
| .await | ||
| .assert_status(200) | ||
| .assert_body_contains("Hello, World!"); | ||
|
|
||
| // Test POST | ||
| client.post_json("/echo", &"Checking echo".to_string()) | ||
| .await | ||
| .assert_status(200) | ||
| .assert_body_contains("Checking echo"); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new integration test provides only basic coverage for GET and POST requests with simple String parameters. There is no test coverage for the new TypedPath and ApiError features that are the main additions in this PR. Consider adding integration tests that verify typed routing works end-to-end and that ApiError-derived enums properly convert to HTTP responses with the correct status codes and error messages.
| } | ||
| } | ||
|
|
||
| /// Add a typed route using a TypedPath |
There was a problem hiding this comment.
The typed method lacks documentation explaining its purpose and providing a usage example. Users may not understand how it differs from the regular route method or how to use it with the TypedPath trait. Add a doc comment with an example showing how to use it with a TypedPath-derived struct.
| /// Add a typed route using a TypedPath | |
| /// Add a typed route using a [`TypedPath`] implementation. | |
| /// | |
| /// This is a convenience for registering routes whose path is defined by a | |
| /// type that implements [`TypedPath`]. Instead of writing the path string | |
| /// manually, the route path is taken from `P::PATH`, which helps keep your | |
| /// handler and the path definition in sync. | |
| /// | |
| /// Internally this is equivalent to calling [`Router::route`] with | |
| /// `P::PATH`: | |
| /// | |
| /// ```ignore | |
| /// router.route(P::PATH, method_router); | |
| /// ``` | |
| /// | |
| /// # Examples | |
| /// | |
| /// ```no_run | |
| /// use http::Method; | |
| /// use rustapi_core::router::{Router, MethodRouter}; | |
| /// use rustapi_core::typed_path::TypedPath; | |
| /// | |
| /// // Define a typed path for a user resource. | |
| /// // | |
| /// // The derive macro provides a `TypedPath` implementation with | |
| /// // `PATH = "/users/{id}"`. | |
| /// #[derive(TypedPath)] | |
| /// #[typed_path("/users/{id}")] | |
| /// struct UserPath { | |
| /// id: String, | |
| /// } | |
| /// | |
| /// // Register a GET handler for `/users/{id}` using the typed path. | |
| /// let router = Router::new().typed::<UserPath>( | |
| /// MethodRouter::new().on(Method::GET, |req| async move { | |
| /// // handle GET /users/{id} | |
| /// }), | |
| /// ); | |
| /// | |
| /// // This is equivalent to: | |
| /// // | |
| /// // let router = Router::new().route( | |
| /// // "/users/{id}", | |
| /// // MethodRouter::new().on(Method::GET, |req| async move { | |
| /// // // handle GET /users/{id} | |
| /// // }), | |
| /// // ); | |
| /// ``` |
| fn to_uri(&self) -> String { | ||
| format!(#format_string, #(#format_args),*) | ||
| } |
There was a problem hiding this comment.
The generated to_uri method does not perform URL encoding of path parameters. If a user passes a parameter value containing special characters like spaces, slashes, or query separators (e.g., "/", "?", "&", " "), the generated URL will be malformed or potentially exploitable. Consider using proper URL encoding (percent-encoding) for the parameter values in the generated code, or document that users must ensure parameters are URL-safe.
| // Re-export TypedPath derive macro | ||
| pub use rustapi_macros::TypedPath; |
There was a problem hiding this comment.
The ApiError derive macro is not exported in the prelude, making it unavailable to users. While the macro is defined in rustapi-macros and demonstrated in README and tests, it's missing from the prelude exports. Add pub use rustapi_macros::ApiError; after line 293 where TypedPath is exported, to make this feature accessible to users who import from the prelude.
| // Re-export TypedPath derive macro | |
| pub use rustapi_macros::TypedPath; | |
| // Re-export derive macros from rustapi_macros | |
| pub use rustapi_macros::TypedPath; | |
| pub use rustapi_macros::ApiError; |
crates/rustapi-macros/src/lib.rs
Outdated
| let code = attr_info | ||
| .as_ref() | ||
| .and_then(|i| i.code.clone()) | ||
| .unwrap_or_else(|| { | ||
| // Default code is snake_case of variant name | ||
| // This is a naive implementation, real world might want a proper snake_case conversion library | ||
| variant_name.to_string().to_lowercase() | ||
| }); |
There was a problem hiding this comment.
The naive lowercase conversion for the default error code doesn't properly convert PascalCase/CamelCase to snake_case. For example, "UserNotFound" becomes "usernotfound" instead of "user_not_found", and "InvalidInput" becomes "invalidinput" instead of "invalid_input". Consider using a proper snake_case conversion (e.g., inserting underscores before uppercase letters) or using an existing crate like heck for proper case conversion.
Moved and refactored the ApiError derive macro implementation into a dedicated module in rustapi-macros, simplifying and improving its logic. Updated imports and re-exports to support the new macro location. Added rustapi-testing as a dev-dependency and updated test code to use it. Minor code cleanups and #[allow(dead_code)] annotations were added to suppress warnings in test and property test code. Removed the semver check job from the CI workflow.
Updated rustapi-extras to depend on rustapi-testing and refactored CSRF layer tests to import TestClient, TestRequest, and TestResponse from rustapi-testing instead of rustapi-core. This improves test modularity and aligns with the new crate structure.
…amework-ergonomics-update Add TypedPath and ApiError derive macros with typed routing 87f05a2
This pull request adds first-class support for typed, type-safe path routing and error handling to the framework, making it easier for users to define, register, and generate URLs for routes using strongly-typed structs. It introduces a new
TypedPathtrait and derive macro, aTypedextractor, and a newApiErrorderive macro for ergonomic error handling. The changes also expose several previously-internal APIs and add a new example demonstrating the feature.Typed Path Routing and Extraction:
TypedPathtrait and aTypedPathderive macro, allowing users to define structs that represent route parameters and automatically generate path patterns and URL builders. (crates/rustapi-core/src/typed_path.rs[1]crates/rustapi-macros/src/lib.rs[2]crates/rustapi-rs/src/lib.rs[3] [4]Typed<T>extractor, enabling handlers to receive path parameters as strongly-typed structs. (crates/rustapi-core/src/extract.rs[1] [2].typed::<P>()methods to bothRustApiandRouterfor registering typed routes, and re-exported relevant types in the public API. (crates/rustapi-core/src/app.rs[1]crates/rustapi-core/src/router.rs[2] [3] [4] [5]Ergonomic Error Handling:
ApiErrorderive macro, allowing users to declaratively define error enums with status codes, error codes, and messages using attributes. (crates/rustapi-macros/src/lib.rscrates/rustapi-macros/src/lib.rsR1094-R1350)README.mdREADME.mdR214-R238)API and Usability Improvements:
BodyVariant,Request::new,Router::match_route,Router::state_ref,RouteMatch) to allow for more flexible integrations and advanced use cases. (crates/rustapi-core/src/request.rs[1] [2]crates/rustapi-core/src/router.rs[3] [4] [5]typed_path_poc.rs) demonstrating typed path usage and type-safe URL generation. (crates/rustapi-rs/examples/typed_path_poc.rscrates/rustapi-rs/examples/typed_path_poc.rsR1-R56)Other:
crates/rustapi-extras/src/cors/mod.rscrates/rustapi-extras/src/cors/mod.rsL198-R204)These changes make the framework more ergonomic, type-safe, and user-friendly, especially for building APIs with complex routing and error handling requirements.