From 6b236703a227dedd15d6888b12a964b593796c26 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 28 Jan 2025 12:54:59 -0500 Subject: [PATCH 1/6] refactor: improve and document handler type inference --- src/lib.rs | 4 +- src/routes/handler.rs | 408 ++++++++++++++++++++++++++++++++++++++---- src/routes/mod.rs | 2 +- src/types/resp.rs | 12 ++ 4 files changed, 393 insertions(+), 33 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 80a018d..6b1c597 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -144,7 +144,9 @@ pub mod pubsub; pub use pubsub::ReadJsonStream; mod routes; -pub use routes::{BatchFuture, Handler, HandlerArgs, HandlerCtx, NotifyError, RouteFuture}; +pub use routes::{ + BatchFuture, Handler, HandlerArgs, HandlerCtx, NotifyError, Params, RouteFuture, State, +}; pub(crate) use routes::{BoxedIntoRoute, ErasedIntoRoute, Method, Route}; mod router; diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 7e8357e..89abfc7 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -13,6 +13,26 @@ macro_rules! convert_result { }}; } +/// Hint type for differentiating certain handler impls. See the [`Handler`] +/// trait "Handler argument type inference" section for more information. +#[derive(Debug)] +#[repr(transparent)] +pub struct State(S); + +/// Hint type for differentiating certain handler impls. See the [`Handler`] +/// trait "Handler argument type inference" section for more information. +#[derive(Debug)] +#[repr(transparent)] +pub struct Params(T); + +/// Marker type used for differentiating certain handler impls. +#[allow(missing_debug_implementations, unreachable_pub)] +pub struct PhantomState(PhantomData); + +/// Marker type used for differentiating certain handler impls. +#[allow(missing_debug_implementations, unreachable_pub)] +pub struct PhantomParams(PhantomData); + /// A trait describing handlers for JSON-RPC methods. /// /// Handlers map some input type `T` to a future that resolve to a @@ -34,9 +54,48 @@ macro_rules! convert_result { /// }; /// ``` /// +/// ### Handler argument type inference +/// +/// When the following conditions are true, the compiler may fail to infer +/// whether a handler argument is `params` or `state`: +/// +/// 1. The handler takes EITHER `params` or `state` +/// 2. The `S` type of the router is a valid `params` type (i.e. it impls +/// [`serde::de::DeserializeOwned`] and satisfies the other requirements of +/// [`RpcRecv`]). +/// 3. The argument to the handler matches the `S` type of the router. +/// +/// In these cases the compiler will find multiple valid impls of the `Handler` +/// trait. In order to differentiate these impls, you can use the [`State`] +/// wrapper struct to indicate that the argument is `state`, or the [`Params`] +/// wrapper struct to indicate that the argument is `params`. +/// +/// ```no_run +/// use ajj::{Router, Params, State}; +/// +/// async fn ok() -> Result<(), ()> { Ok(()) } +/// +/// # #[tokio::main] +/// # async fn main() -> Result<(), Box> { +/// Router::new::() +/// // this will fail to infer the `Handler` impl as the argument could be +/// // either `params` or `state` +/// .route("foo", |something: u8| ok()) +/// // this will succeed as the `State` wrapper indicates that the argument +/// // is `state` +/// .route("bar", |State(something): State| ok()) +/// // this will succeed as the `Params` wrapper indicates that the argument +/// // is `params` +/// .route("baz", |Params(something): Params| ok()); +/// # } +/// ``` +/// +/// These wrapper structs are available only when necessary, and may not be +/// used when the `Handler` impl is unambiguous. +/// /// ### Handler return type inference /// -/// Handlers that always suceed or always fail may have trouble with type +/// Handlers that always succeed or always fail may have trouble with type /// inference, as they contain an unknown type parameter, which could be /// anything. Here's an example of code with failed type inference: /// @@ -106,11 +165,12 @@ macro_rules! convert_result { /// [`Result`]: /// /// - `async fn()` -/// - `async fn(Params) -> Fut` -/// - `async fn(HandlerCtx, Params) -> Fut` -/// - `async fn(Params, S) -> Fut` +/// - `async fn(HandlerCtx) -> Fut` /// - `async fn(HandlerCtx, Params) -> Fut` +/// - `async fn(HandlerCtx, S) -> Fut` /// - `async fn(HandlerCtx, Params, S) -> Fut` +/// - `async fn(Params) -> Fut` +/// - `async fn(Params, S) -> Fut` /// /// ### Implementer's note: /// @@ -305,11 +365,12 @@ where } } -impl Handler<(OutputResponsePayload, Params), S> for F +impl Handler<(OutputResponsePayload, PhantomParams), S> + for F where - F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(Input) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, { @@ -329,12 +390,74 @@ where } } -impl Handler<(OutputResponsePayload, HandlerCtx, Params), S> - for F +impl Handler<(OutputResponsePayload, Params), S> for F where - F: FnOnce(HandlerCtx, Params) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, + Payload: RpcSend, + ErrData: RpcSend, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { + let HandlerArgs { req, .. } = args; + Box::pin(async move { + let id = req.id_owned(); + let Ok(params) = req.deser_params() else { + return Response::maybe_invalid_params(id.as_deref()); + }; + + let payload = self(Params(params)).await; + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResponsePayload, PhantomState), S> for F +where + F: FnOnce(S) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + let id = args.req.id_owned(); + Box::pin(async move { + let payload = self(state).await; + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResponsePayload, State), S> for F +where + F: FnOnce(State) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + let id = args.req.id_owned(); + Box::pin(async move { + let payload = self(State(state)).await; + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl + Handler<(OutputResponsePayload, HandlerCtx, PhantomParams), S> for F +where + F: FnOnce(HandlerCtx, Input) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, { @@ -357,11 +480,39 @@ where } } -impl Handler<(OutputResponsePayload, Params, S), S> for F +impl + Handler<(OutputResponsePayload, HandlerCtx, Params), S> for F +where + F: FnOnce(HandlerCtx, Params) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Input: RpcRecv, + Payload: RpcSend, + ErrData: RpcSend, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { + Box::pin(async move { + let HandlerArgs { ctx, req } = args; + + let id = req.id_owned(); + let Ok(params) = req.deser_params() else { + return Response::maybe_invalid_params(id.as_deref()); + }; + + drop(req); // deallocate explicitly. No funny business. + + let payload = self(ctx, Params(params)).await; + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResponsePayload, Input, S), S> for F where - F: FnOnce(Params, S) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(Input, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, S: Send + Sync + 'static, @@ -385,7 +536,8 @@ where } } -impl Handler<(OutputResponsePayload, S, HandlerCtx), S> for F +impl Handler<(OutputResponsePayload, HandlerCtx, PhantomState), S> + for F where F: FnOnce(HandlerCtx, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, @@ -410,12 +562,37 @@ where } } -impl Handler<(OutputResponsePayload, HandlerCtx, Params, S), S> +impl Handler<(OutputResponsePayload, HandlerCtx, State), S> for F +where + F: FnOnce(HandlerCtx, State) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + Box::pin(async move { + let HandlerArgs { ctx, req } = args; + + let id = req.id_owned(); + + drop(req); // deallocate explicitly. No funny business. + + let payload = self(ctx, State(state)).await; + + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResponsePayload, HandlerCtx, Input, S), S> for F where - F: FnOnce(HandlerCtx, Params, S) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(HandlerCtx, Input, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, S: Send + Sync + 'static, @@ -481,11 +658,11 @@ where } } -impl Handler<(OutputResult, Params), S> for F +impl Handler<(OutputResult, PhantomParams), S> for F where - F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(Input) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, { @@ -509,11 +686,78 @@ where } } -impl Handler<(OutputResult, HandlerCtx, Params), S> for F +impl Handler<(OutputResult, Params), S> for F +where + F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Input: RpcRecv, + Payload: RpcSend, + ErrData: RpcSend, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { + Box::pin(async move { + let HandlerArgs { req, .. } = args; + + let id = req.id_owned(); + let Ok(params) = req.deser_params() else { + return Response::maybe_invalid_params(id.as_deref()); + }; + + drop(req); // deallocate explicitly. No funny business. + + let payload = convert_result!(self(Params(params)).await); + + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResult, PhantomState), S> for F +where + F: FnOnce(S) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + let id = args.req.id_owned(); + Box::pin(async move { + let payload = convert_result!(self(state).await); + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResult, State), S> for F +where + F: FnOnce(State) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + let id = args.req.id_owned(); + Box::pin(async move { + let payload = convert_result!(self(State(state)).await); + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl + Handler<(OutputResult, HandlerCtx, PhantomParams), S> for F where - F: FnOnce(HandlerCtx, Params) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(HandlerCtx, Input) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, { @@ -537,11 +781,39 @@ where } } -impl Handler<(OutputResult, Params, S), S> for F +impl Handler<(OutputResult, HandlerCtx, Params), S> for F +where + F: FnOnce(HandlerCtx, Params) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Input: RpcRecv, + Payload: RpcSend, + ErrData: RpcSend, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { + Box::pin(async move { + let HandlerArgs { ctx, req } = args; + + let id = req.id_owned(); + let Ok(params) = req.deser_params() else { + return Response::maybe_invalid_params(id.as_deref()); + }; + + drop(req); // deallocate explicitly. No funny business. + + let payload = convert_result!(self(ctx, Params(params)).await); + + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResult, Input, S), S> for F where - F: FnOnce(Params, S) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(Input, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, S: Send + Sync + 'static, @@ -566,7 +838,7 @@ where } } -impl Handler<(OutputResult, S, HandlerCtx), S> for F +impl Handler<(OutputResult, HandlerCtx, PhantomState), S> for F where F: FnOnce(HandlerCtx, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, @@ -591,11 +863,36 @@ where } } -impl Handler<(OutputResult, HandlerCtx, Params, S), S> for F +impl Handler<(OutputResult, HandlerCtx, State), S> for F +where + F: FnOnce(HandlerCtx, State) -> Fut + Clone + Send + Sync + 'static, + Fut: Future> + Send + 'static, + Payload: RpcSend, + ErrData: RpcSend, + S: Send + Sync + 'static, +{ + type Future = Pin>> + Send>>; + + fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { + let HandlerArgs { ctx, req } = args; + + let id = req.id_owned(); + + drop(req); + + Box::pin(async move { + let payload = convert_result!(self(ctx, State(state)).await); + + Response::maybe(id.as_deref(), &payload) + }) + } +} + +impl Handler<(OutputResult, HandlerCtx, Input, S), S> for F where - F: FnOnce(HandlerCtx, Params, S) -> Fut + Clone + Send + Sync + 'static, + F: FnOnce(HandlerCtx, Input, S) -> Fut + Clone + Send + Sync + 'static, Fut: Future> + Send + 'static, - Params: RpcRecv, + Input: RpcRecv, Payload: RpcSend, ErrData: RpcSend, S: Send + Sync + 'static, @@ -620,6 +917,55 @@ where } } +#[cfg(test)] +mod test { + use super::*; + use crate::{HandlerCtx, ResponsePayload}; + + #[derive(Clone)] + struct NewType; + + async fn resp_ok() -> ResponsePayload<(), ()> { + ResponsePayload::Success(()) + } + + async fn ok() -> Result<(), ()> { + Ok(()) + } + + #[test] + #[ignore = "compilation_only"] + #[allow(unused_variables)] + fn combination_inference() { + let router: crate::Router<()> = crate::Router::::new() + //responses + .route("respnse", || ok()) + .route("respnse, ctx", |_: HandlerCtx| resp_ok()) + .route("respnse, params", |_: u16| resp_ok()) + .route("respnse, ctx, params", |_: HandlerCtx, _: u16| resp_ok()) + .route("respnse, params, state", |_: u8, _: NewType| resp_ok()) + .route("respnse, ctx, state", |_: HandlerCtx, _: NewType| resp_ok()) + .route( + "respnse, ctx, params, state", + |_: HandlerCtx, _: u8, _: NewType| resp_ok(), + ) + // results + .route("result", || ok()) + .route("result, ctx", |_: HandlerCtx| ok()) + .route("result, params", |_: u16| ok()) + .route("result, ctx, params", |_: HandlerCtx, _: u16| ok()) + .route("result, params, state", |_: u8, _: NewType| ok()) + .route("result, ctx, state", |_: HandlerCtx, _: NewType| ok()) + .route( + "result, ctx, params, state", + |_: HandlerCtx, _: u8, _: NewType| ok(), + ) + .with_state::(NewType) + .route::<_, (OutputResult, State)>("no inference error", |State(state)| ok()) + .with_state(1u8); + } +} + // Some code is this file is reproduced under the terms of the MIT license. It // originates from the `axum` crate. The original source code can be found at // the following URL, and the original license is included below. diff --git a/src/routes/mod.rs b/src/routes/mod.rs index ef5499f..a0bd53b 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -8,8 +8,8 @@ mod future; pub use future::{BatchFuture, RouteFuture}; mod handler; -pub use handler::Handler; pub(crate) use handler::HandlerInternal; +pub use handler::{Handler, Params, State}; mod method; pub(crate) use method::Method; diff --git a/src/types/resp.rs b/src/types/resp.rs index 27dd6db..3b2fe4e 100644 --- a/src/types/resp.rs +++ b/src/types/resp.rs @@ -203,6 +203,18 @@ impl ResponsePayload { pub const fn is_error(&self) -> bool { matches!(self, Self::Failure(_)) } + + /// Convert a result into a response payload, by converting the error into + /// an internal error message. + pub fn convert_internal_error_msg(res: Result) -> Self + where + T: Into>, + { + match res { + Ok(payload) => Self::Success(payload), + Err(err) => Self::Failure(ErrorPayload::internal_error_message(err.into())), + } + } } /// A JSON-RPC 2.0 error object. From e9590dbf956271e71f59fd30b16807de300923e3 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 28 Jan 2025 13:02:38 -0500 Subject: [PATCH 2/6] fix: clippy and tests --- src/lib.rs | 4 ++-- src/router.rs | 6 ++++-- src/routes/handler.rs | 17 +++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6b1c597..4421d86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,12 +10,12 @@ //! register JSON-RPC methods and their handlers. //! //! ```no_run -//! use ajj::{Router, HandlerCtx, ResponsePayload}; +//! use ajj::{Router, HandlerCtx, ResponsePayload, Params}; //! //! # fn main() { //! // Provide methods called "double" and "add" to the router. //! let router = Router::::new() -//! .route("double", |params: u64| async move { +//! .route("double", |Params(params): Params| async move { //! Ok::<_, ()>(params * 2) //! }) //! .route("add", |params: u64, state: u64| async move { diff --git a/src/router.rs b/src/router.rs index 8a1da30..137eb9c 100644 --- a/src/router.rs +++ b/src/router.rs @@ -36,12 +36,14 @@ use tracing::{debug_span, trace}; /// These methods can /// /// ``` -/// use ajj::{Router}; +/// use ajj::{Router, Params}; /// /// # fn main() { /// // Provide methods called "double" and "add" to the router. /// let router = Router::::new() -/// .route("double", |params: u64| async move { +/// // when double is called, the provided number is doubled. +/// // see the Handler docs for more information on the hint type +/// .route("double", |Params(params): Params| async move { /// Ok::<_, ()>(params * 2) /// }) /// .route("add", |params: u64, state: u64| async move { diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 89abfc7..317382f 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -17,13 +17,13 @@ macro_rules! convert_result { /// trait "Handler argument type inference" section for more information. #[derive(Debug)] #[repr(transparent)] -pub struct State(S); +pub struct State(pub S); /// Hint type for differentiating certain handler impls. See the [`Handler`] /// trait "Handler argument type inference" section for more information. #[derive(Debug)] #[repr(transparent)] -pub struct Params(T); +pub struct Params(pub T); /// Marker type used for differentiating certain handler impls. #[allow(missing_debug_implementations, unreachable_pub)] @@ -59,7 +59,7 @@ pub struct PhantomParams(PhantomData); /// When the following conditions are true, the compiler may fail to infer /// whether a handler argument is `params` or `state`: /// -/// 1. The handler takes EITHER `params` or `state` +/// 1. The handler takes EITHER `params` or `state`. /// 2. The `S` type of the router is a valid `params` type (i.e. it impls /// [`serde::de::DeserializeOwned`] and satisfies the other requirements of /// [`RpcRecv`]). @@ -70,16 +70,17 @@ pub struct PhantomParams(PhantomData); /// wrapper struct to indicate that the argument is `state`, or the [`Params`] /// wrapper struct to indicate that the argument is `params`. /// -/// ```no_run +/// ```compile_fail /// use ajj::{Router, Params, State}; /// /// async fn ok() -> Result<(), ()> { Ok(()) } /// -/// # #[tokio::main] -/// # async fn main() -> Result<(), Box> { +/// # fn main() -> Result<(), Box> { /// Router::new::() /// // this will fail to infer the `Handler` impl as the argument could be /// // either `params` or `state` +/// // The error will look like this: +/// // cannot infer type for type parameter `T` declared on the method `route` /// .route("foo", |something: u8| ok()) /// // this will succeed as the `State` wrapper indicates that the argument /// // is `state` @@ -939,7 +940,7 @@ mod test { fn combination_inference() { let router: crate::Router<()> = crate::Router::::new() //responses - .route("respnse", || ok()) + .route("respnse", ok) .route("respnse, ctx", |_: HandlerCtx| resp_ok()) .route("respnse, params", |_: u16| resp_ok()) .route("respnse, ctx, params", |_: HandlerCtx, _: u16| resp_ok()) @@ -950,7 +951,7 @@ mod test { |_: HandlerCtx, _: u8, _: NewType| resp_ok(), ) // results - .route("result", || ok()) + .route("result", ok) .route("result, ctx", |_: HandlerCtx| ok()) .route("result, params", |_: u16| ok()) .route("result, ctx, params", |_: HandlerCtx, _: u16| ok()) From d3fb3ff36b28ead697df88a635b6e797340022b0 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 28 Jan 2025 16:18:28 -0500 Subject: [PATCH 3/6] docs: more info in the handler docs --- Cargo.toml | 2 +- src/lib.rs | 16 ++++---- src/router.rs | 19 ++++----- src/routes/handler.rs | 91 ++++++++++++++++++++++++++++++++++--------- 4 files changed, 89 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1d6f9fd..8ce47be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ description = "Simple, modern, ergonomic JSON-RPC 2.0 router built with tower an keywords = ["json-rpc", "jsonrpc", "json"] categories = ["web-programming::http-server", "web-programming::websocket"] -version = "0.1.2" +version = "0.2.0" edition = "2021" rust-version = "1.81" authors = ["init4", "James Prestwich"] diff --git a/src/lib.rs b/src/lib.rs index 4421d86..73dfb67 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,17 +10,18 @@ //! register JSON-RPC methods and their handlers. //! //! ```no_run -//! use ajj::{Router, HandlerCtx, ResponsePayload, Params}; +//! use ajj::{Router, HandlerCtx, ResponsePayload}; //! -//! # fn main() { +//! # fn test_fn() -> Router<()> { //! // Provide methods called "double" and "add" to the router. //! let router = Router::::new() -//! .route("double", |Params(params): Params| async move { -//! Ok::<_, ()>(params * 2) -//! }) //! .route("add", |params: u64, state: u64| async move { //! Ok::<_, ()>(params + state) //! }) +//! .with_state(3u64) +//! .route("double", |params: u64| async move { +//! Ok::<_, ()>(params * 2) +//! }) //! // Routes get a ctx, which can be used to send notifications. //! .route("notify", |ctx: HandlerCtx| async move { //! if ctx.notifications().is_none() { @@ -43,9 +44,8 @@ //! .route("error_example", || async { //! // This will appear in the ResponsePayload's `message` field. //! ResponsePayload::<(), ()>::internal_error_message("this is an error".into()) -//! }) -//! // The router is provided with state, and is now ready to serve requests. -//! .with_state::<()>(3u64); +//! }); +//! # router //! # } //! ``` //! diff --git a/src/router.rs b/src/router.rs index 137eb9c..490e189 100644 --- a/src/router.rs +++ b/src/router.rs @@ -33,28 +33,25 @@ use tracing::{debug_span, trace}; /// The `double` method doubles the provided number, and the `add` method adds /// the provided number to some state, and returns the sum. /// -/// These methods can -/// /// ``` -/// use ajj::{Router, Params}; +/// use ajj::Router; /// /// # fn main() { /// // Provide methods called "double" and "add" to the router. /// let router = Router::::new() -/// // when double is called, the provided number is doubled. -/// // see the Handler docs for more information on the hint type -/// .route("double", |Params(params): Params| async move { -/// Ok::<_, ()>(params * 2) -/// }) +/// // This route requires state to be provided. /// .route("add", |params: u64, state: u64| async move { /// Ok::<_, ()>(params + state) /// }) -/// // The router is provided with state, and is now ready to serve requests. -/// .with_state::<()>(3u64); +/// .with_state::<()>(3u64) +/// // when double is called, the provided number is doubled. +/// // see the Handler docs for more information on the hint type +/// .route("double", |params: u64| async move { +/// Ok::<_, ()>(params * 2) +/// }); /// # } /// ``` /// -/// /// ## Note /// /// The state `S` is "missing" state. It is state that must be added to the diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 317382f..14f519c 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -65,29 +65,82 @@ pub struct PhantomParams(PhantomData); /// [`RpcRecv`]). /// 3. The argument to the handler matches the `S` type of the router. /// -/// In these cases the compiler will find multiple valid impls of the `Handler` -/// trait. In order to differentiate these impls, you can use the [`State`] -/// wrapper struct to indicate that the argument is `state`, or the [`Params`] -/// wrapper struct to indicate that the argument is `params`. -/// /// ```compile_fail -/// use ajj::{Router, Params, State}; +/// use ajj::Router; /// /// async fn ok() -> Result<(), ()> { Ok(()) } /// -/// # fn main() -> Result<(), Box> { -/// Router::new::() -/// // this will fail to infer the `Handler` impl as the argument could be -/// // either `params` or `state` -/// // The error will look like this: -/// // cannot infer type for type parameter `T` declared on the method `route` -/// .route("foo", |something: u8| ok()) -/// // this will succeed as the `State` wrapper indicates that the argument -/// // is `state` -/// .route("bar", |State(something): State| ok()) -/// // this will succeed as the `Params` wrapper indicates that the argument -/// // is `params` -/// .route("baz", |Params(something): Params| ok()); +/// # fn test_fn() -> Router<()> { +/// // These will fail to infer the `Handler` impl as the argument could be +/// // either `params` or `state`. +/// // +/// // The error will look like this: +/// // cannot infer type for type parameter `T` declared on the method +/// // `route` +/// Router::::new() +/// // "foo" and "bar" are ambiguous, as the `S` type is `u8` +/// .route("foo", |params: u8| ok()) +/// .route("bar", |state: u8| ok()) +/// // "baz" is unambiguous, as it has both `params` and `state` arguments +/// .route("baz", |params: u8, state: u8| ok()) +/// .with_state(3u8) +/// # } +/// ``` +/// +/// In these cases the compiler will find multiple valid impls of the `Handler` +/// trait. +/// +/// The easiest way to resolve this is to avoid using the `S` type of the +/// router as the `params` argument to the handler. This will ensure that the +/// compiler can always infer the correct impl of the `Handler` trait. In cases +/// where `Params` and `S` must be the same type, you can use the [`Params`] and +/// [`State`] wrapper structs in your handler arguments to disambiguate the +/// argument type. +/// +/// The usual way to fix this is to reorganize route invocations to avoid the +/// ambiguity: +/// +/// ``` +/// # use ajj::{Router, Params, State}; +/// # async fn ok() -> Result<(), ()> { Ok(()) } +/// # fn test_fn() -> Router<()> { +/// // When "foo" is routed, the argument is unambiguous, as the `S` type +/// // is no longer `u8`. +/// Router::::new() +/// // This was already unambiguous, as having both `params` and `state` is +/// // never ambiguous +/// .route("baz", |params: u8, state: u8| ok()) +/// .with_state::<()>(3u8) +/// +/// // "bar" presents a problem. We'll come back to this in the next +/// // example. +/// // .route("bar", |state: u8| ok()) +/// +/// // `S` is now `()`, so the argument is unambiguous. +/// .route("foo", |params: u8| ok()) +/// # } +/// ``` +/// +/// However this still leaves the problem of "bar". There is no way to express +/// "bar" unambiguously by reordering method invocations. In this case, you can +/// use the [`Params`] and [`State`] wrapper structs to disambiguate the +/// argument type. +/// +/// ``` +/// # use ajj::{Router, Params, State}; +/// # async fn ok() -> Result<(), ()> { Ok(()) } +/// # fn test_fn() -> Router<()> { +/// Router::::new() +/// // This is now unambiguous, as the `Params` wrapper indicates that the +/// // argument is `params` +/// .route("foo", |Params(params): Params| ok()) +/// // This is now umabiguous, as the `State` wrapper indicates that the +/// // argument is `state` +/// .route("bar", |State(state): State| ok()) +/// // This was already unambiguous, as having both `params` and `state` is +/// // never ambiguous +/// .route("baz", |params: u8, state: u8| ok()) +/// .with_state::<()>(3u8) /// # } /// ``` /// From 90da49a9effd0d8254b1ab3482e4587ed1d65307 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 28 Jan 2025 16:21:58 -0500 Subject: [PATCH 4/6] doc: fix em --- src/routes/future.rs | 9 +++++---- src/routes/handler.rs | 2 ++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/routes/future.rs b/src/routes/future.rs index fc65c4c..84004ca 100644 --- a/src/routes/future.rs +++ b/src/routes/future.rs @@ -13,10 +13,9 @@ use tower::util::{BoxCloneSyncService, Oneshot}; use super::Response; -/// A future produced by [`Router::call_with_state`]. This should only be -/// instantiated by that method. +/// A future produced by the [`Router`]. /// -/// [`Route`]: crate::routes::Route +/// [`Router`]: crate::Router #[pin_project] pub struct RouteFuture { /// The inner [`Route`] future. @@ -159,7 +158,9 @@ impl BatchFuture { } /// Push a parse result into the batch. Convenience function to simplify - /// [`Router::call_batch_with_state`] logic. + /// [`Router`] logic. + /// + /// [`Router`]: crate::Router pub(crate) fn push_parse_result(&mut self, result: Result) { match result { Ok(fut) => self.push(fut), diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 14f519c..0cb5120 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -134,9 +134,11 @@ pub struct PhantomParams(PhantomData); /// // This is now unambiguous, as the `Params` wrapper indicates that the /// // argument is `params` /// .route("foo", |Params(params): Params| ok()) +/// /// // This is now umabiguous, as the `State` wrapper indicates that the /// // argument is `state` /// .route("bar", |State(state): State| ok()) +/// /// // This was already unambiguous, as having both `params` and `state` is /// // never ambiguous /// .route("baz", |params: u8, state: u8| ok()) From d86d35b9a8c10e2a110925849aab5c55dfc9add2 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 28 Jan 2025 16:25:58 -0500 Subject: [PATCH 5/6] doc: nits --- src/routes/handler.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 0cb5120..ce59f10 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -60,9 +60,9 @@ pub struct PhantomParams(PhantomData); /// whether a handler argument is `params` or `state`: /// /// 1. The handler takes EITHER `params` or `state`. -/// 2. The `S` type of the router is a valid `params` type (i.e. it impls -/// [`serde::de::DeserializeOwned`] and satisfies the other requirements of -/// [`RpcRecv`]). +/// 2. The `S` type of the router would also be a valid `Params` type (i.e. it +/// impls [`DeserializeOwned`] and satisfies the other +/// requirements of [`RpcRecv`]). /// 3. The argument to the handler matches the `S` type of the router. /// /// ```compile_fail @@ -110,12 +110,13 @@ pub struct PhantomParams(PhantomData); /// // This was already unambiguous, as having both `params` and `state` is /// // never ambiguous /// .route("baz", |params: u8, state: u8| ok()) -/// .with_state::<()>(3u8) /// /// // "bar" presents a problem. We'll come back to this in the next /// // example. /// // .route("bar", |state: u8| ok()) /// +/// .with_state::<()>(3u8) +/// /// // `S` is now `()`, so the argument is unambiguous. /// .route("foo", |params: u8| ok()) /// # } @@ -280,6 +281,8 @@ pub struct PhantomParams(PhantomData); /// } /// } /// ``` +/// +/// [`DeserializeOwned`]: serde::de::DeserializeOwned #[cfg_attr(feature = "pubsub", doc = " [`Listener`]: crate::pubsub::Listener")] pub trait Handler: Clone + Send + Sync + Sized + 'static { /// The future returned by the handler. From e264c218a3f66fba1f582064bd8d1ec85700eeb7 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 29 Jan 2025 08:29:35 -0500 Subject: [PATCH 6/6] docs: slight improvements --- src/routes/handler.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/routes/handler.rs b/src/routes/handler.rs index ce59f10..6b9bb62 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -83,6 +83,8 @@ pub struct PhantomParams(PhantomData); /// .route("bar", |state: u8| ok()) /// // "baz" is unambiguous, as it has both `params` and `state` arguments /// .route("baz", |params: u8, state: u8| ok()) +/// // this is unambiguous, but a bit ugly. +/// .route("qux", |params: u8, _: u8| ok()) /// .with_state(3u8) /// # } /// ``` @@ -125,7 +127,7 @@ pub struct PhantomParams(PhantomData); /// However this still leaves the problem of "bar". There is no way to express /// "bar" unambiguously by reordering method invocations. In this case, you can /// use the [`Params`] and [`State`] wrapper structs to disambiguate the -/// argument type. +/// argument type. This should seem familiar to users of [`axum`]. /// /// ``` /// # use ajj::{Router, Params, State}; @@ -148,7 +150,8 @@ pub struct PhantomParams(PhantomData); /// ``` /// /// These wrapper structs are available only when necessary, and may not be -/// used when the `Handler` impl is unambiguous. +/// used when the `Handler` impl is unambiguous. Ambiguous handlers will always +/// result in a compilation error. /// /// ### Handler return type inference ///