Handler metadata + EndpointResolver Fix (#98)

* Added handler name to method metadata

* Bugfix/endpoint ordering (#97)

Fixed Endpoint ordering.

* Fixed issue with resolver + potential issue with Method collision

* Cargo fmt

* Saphir v2.7

Co-authored-by: Samuel Bergeron-Drouin <[email protected]>

saphir/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "saphir"
-version = "2.6.12"
+version = "2.7.0"
 edition = "2018"
 authors = ["Richer Archambault <[email protected]>"]
 description = "Fully async-await http server framework"
@@ -37,13 +37,13 @@ http-body = "0.3"
 parking_lot = "0.11"
 regex = "1.3"
 uuid = { version = "0.8", features = ["serde", "v4"], optional = true }
-rustls = { version = "0.17", optional = true }
-tokio-rustls = { version = "0.13", optional = true }
+rustls = { version = "0.18", optional = true }
+tokio-rustls = { version = "0.14", optional = true }
 base64 = { version = "0.12", optional = true }
 serde = { version = "1.0", optional = true }
 serde_json = { version = "1.0", optional = true }
 serde_urlencoded = { version = "0.6", optional = true }
-saphir_macro = { path = "../saphir_macro", version = "2.0.10", optional = true }
+saphir_macro = { path = "../saphir_macro", version = "2.1", optional = true }
 mime = { version = "0.3", optional = true }
 nom = { version = "5", optional = true }
 mime_guess = { version = "2.0.3", optional = true }

saphir/examples/macro.rs

@@ -101,6 +101,7 @@ impl ApiKeyMiddleware {
             info!("Not Authenticated");
         }
 
+        info!("Handler {} will be used", ctx.metadata.name.unwrap_or("unknown"));
         chain.next(ctx).await
     }
 }

saphir/src/controller.rs

@@ -42,7 +42,13 @@ use futures_util::future::{Future, FutureExt};
 use http::Method;
 
 /// Type definition to represent a endpoint within a controller
-pub type ControllerEndpoint<C> = (Method, &'static str, Box<dyn DynControllerHandler<C, Body> + Send + Sync>, Box<dyn GuardChain>);
+pub type ControllerEndpoint<C> = (
+    Option<&'static str>,
+    Method,
+    &'static str,
+    Box<dyn DynControllerHandler<C, Body> + Send + Sync>,
+    Box<dyn GuardChain>,
+);
 
 /// Trait that defines how a controller handles its requests
 pub trait Controller {
@@ -124,7 +130,7 @@ impl<C: Controller> EndpointsBuilder<C> {
     where
         H: 'static + DynControllerHandler<C, Body> + Send + Sync,
     {
-        self.handlers.push((method, route, Box::new(handler), GuardBuilder::default().build()));
+        self.handlers.push((None, method, route, Box::new(handler), GuardBuilder::default().build()));
         self
     }
 
@@ -136,7 +142,32 @@ impl<C: Controller> EndpointsBuilder<C> {
         F: FnOnce(GuardBuilder<GuardChainEnd>) -> GuardBuilder<Chain>,
         Chain: GuardChain + 'static,
     {
-        self.handlers.push((method, route, Box::new(handler), guards(GuardBuilder::default()).build()));
+        self.handlers
+            .push((None, method, route, Box::new(handler), guards(GuardBuilder::default()).build()));
+        self
+    }
+
+    /// Add but with a handler name
+    #[inline]
+    pub fn add_with_name<H>(mut self, handler_name: &'static str, method: Method, route: &'static str, handler: H) -> Self
+    where
+        H: 'static + DynControllerHandler<C, Body> + Send + Sync,
+    {
+        self.handlers
+            .push((Some(handler_name), method, route, Box::new(handler), GuardBuilder::default().build()));
+        self
+    }
+
+    /// Add with guard but with a handler name
+    #[inline]
+    pub fn add_with_guards_and_name<H, F, Chain>(mut self, handler_name: &'static str, method: Method, route: &'static str, handler: H, guards: F) -> Self
+    where
+        H: 'static + DynControllerHandler<C, Body> + Send + Sync,
+        F: FnOnce(GuardBuilder<GuardChainEnd>) -> GuardBuilder<Chain>,
+        Chain: GuardChain + 'static,
+    {
+        self.handlers
+            .push((Some(handler_name), method, route, Box::new(handler), guards(GuardBuilder::default()).build()));
         self
     }
 

saphir/src/http_context.rs

@@ -147,6 +147,13 @@ impl State {
     }
 }
 
+/// MetaData of the resolved request handler
+#[derive(Debug, Clone, Eq, PartialEq, Default)]
+pub struct HandlerMetadata {
+    pub route_id: u64,
+    pub name: Option<&'static str>,
+}
+
 /// Context representing the relationship between a request and a response
 /// This structure only appears inside Middleware since the act before and after
 /// the request
@@ -161,16 +168,17 @@ pub struct HttpContext {
     #[cfg(feature = "operation")]
     /// Unique Identifier of the current request->response chain
     pub operation_id: crate::http_context::operation::OperationId,
+    pub metadata: HandlerMetadata,
     pub(crate) router: Option<Router>,
 }
 
 impl HttpContext {
-    pub(crate) fn new(request: Request, router: Router) -> Self {
+    pub(crate) fn new(request: Request, router: Router, metadata: HandlerMetadata) -> Self {
         #[cfg(not(feature = "operation"))]
         {
             let state = State::Before(Box::new(request));
             let router = Some(router);
-            HttpContext { state, router }
+            HttpContext { state, metadata, router }
         }
 
         #[cfg(feature = "operation")]
@@ -186,7 +194,12 @@ impl HttpContext {
             *request.operation_id_mut() = operation_id;
             let state = State::Before(Box::new(request));
             let router = Some(router);
-            HttpContext { state, router, operation_id }
+            HttpContext {
+                state,
+                router,
+                operation_id,
+                metadata,
+            }
         }
     }
 

saphir/src/middleware.rs

@@ -172,7 +172,7 @@ impl MiddlewareChain for MiddleChainEnd {
     fn next(&self, mut ctx: HttpContext) -> BoxFuture<'static, Result<HttpContext, SaphirError>> {
         async {
             let router = ctx.router.take().ok_or_else(|| SaphirError::Internal(InternalError::Stack))?;
-            router.handle(ctx).await
+            router.dispatch(ctx).await
         }
         .boxed()
     }

saphir/src/router.rs

@@ -15,7 +15,7 @@ use crate::{
     error::SaphirError,
     guard::{Builder as GuardBuilder, GuardChain, GuardChainEnd},
     handler::DynHandler,
-    http_context::{HttpContext, State},
+    http_context::{HandlerMetadata, HttpContext, State},
     request::Request,
     responder::{DynResponder, Responder},
     utils::{EndpointResolver, EndpointResolverResult},
@@ -140,13 +140,14 @@ impl<Controllers: 'static + RouterChain + Unpin + Send + Sync> Builder<Controlle
     /// ```
     pub fn controller<C: Controller + Send + Unpin + Sync>(mut self, controller: C) -> Builder<RouterChainLink<C, Controllers>> {
         let mut handlers = HashMap::new();
-        for (method, subroute, handler, guard_chain) in controller.handlers() {
+        for (name, method, subroute, handler, guard_chain) in controller.handlers() {
             let route = format!("{}{}", C::BASE_PATH, subroute);
+            let meta = name.map(|name| HandlerMetadata { route_id: 0, name: Some(name) });
             let endpoint_id = if let Some(er) = self.resolver.get_mut(&route) {
-                er.add_method(method.clone());
+                er.add_method_with_metadata(method.clone(), meta);
                 er.id()
             } else {
-                let er = EndpointResolver::new(&route, method.clone()).expect("Unable to construct endpoint resolver");
+                let er = EndpointResolver::new_with_metadata(&route, method.clone(), meta).expect("Unable to construct endpoint resolver");
                 let er_id = er.id();
                 self.resolver.insert(route, er);
                 er_id
@@ -168,9 +169,12 @@ impl<Controllers: 'static + RouterChain + Unpin + Send + Sync> Builder<Controlle
     pub(crate) fn build(self) -> Router {
         let Builder { resolver, chain: controllers } = self;
 
+        let mut resolvers: Vec<_> = resolver.into_iter().map(|(_, e)| e).collect();
+        resolvers.sort_unstable();
+
         Router {
             inner: Arc::new(RouterInner {
-                resolvers: resolver.into_iter().map(|(_, e)| e).collect(),
+                resolvers,
                 chain: Box::new(controllers),
             }),
         }
@@ -199,7 +203,7 @@ impl Router {
             match endpoint_resolver.resolve(req) {
                 EndpointResolverResult::InvalidPath => continue,
                 EndpointResolverResult::MethodNotAllowed => method_not_allowed = true,
-                EndpointResolverResult::Match => return Ok(endpoint_resolver.id()),
+                EndpointResolverResult::Match(_) => return Ok(endpoint_resolver.id()),
             }
         }
 
@@ -210,23 +214,31 @@ impl Router {
         }
     }
 
-    pub async fn handle(self, mut ctx: HttpContext) -> Result<HttpContext, SaphirError> {
-        let mut req = ctx.state.take_request().ok_or_else(|| SaphirError::RequestMovedBeforeHandler)?;
-        match self.resolve(&mut req) {
-            Ok(id) => self.dispatch(id, req, ctx).await,
-            Err(status) => {
-                ctx.state = State::After(Box::new(status.respond_with_builder(crate::response::Builder::new(), &ctx).build()?));
-                Ok(ctx)
+    pub fn resolve_metadata(&self, req: &mut Request<Body>) -> Result<&HandlerMetadata, u16> {
+        let mut method_not_allowed = false;
+
+        for endpoint_resolver in &self.inner.resolvers {
+            match endpoint_resolver.resolve(req) {
+                EndpointResolverResult::InvalidPath => continue,
+                EndpointResolverResult::MethodNotAllowed => method_not_allowed = true,
+                EndpointResolverResult::Match(meta) => return Ok(meta),
             }
         }
+
+        if method_not_allowed {
+            Err(405)
+        } else {
+            Err(404)
+        }
     }
 
-    pub async fn dispatch(&self, resolver_id: u64, req: Request<Body>, mut ctx: HttpContext) -> Result<HttpContext, SaphirError> {
+    pub async fn dispatch(&self, mut ctx: HttpContext) -> Result<HttpContext, SaphirError> {
+        let req = ctx.state.take_request().ok_or_else(|| SaphirError::RequestMovedBeforeHandler)?;
         // # SAFETY #
         // The router is initialized in static memory when calling run on Server.
         let static_self = unsafe { std::mem::transmute::<&'_ Self, &'static Self>(self) };
         let b = crate::response::Builder::new();
-        let res = if let Some(responder) = static_self.inner.chain.dispatch(resolver_id, req) {
+        let res = if let Some(responder) = static_self.inner.chain.dispatch(ctx.metadata.route_id, req) {
             responder.await.dyn_respond(b, &ctx)
         } else {
             404.respond_with_builder(b, &ctx)

saphir/src/server.rs

@@ -27,7 +27,7 @@ use crate::{
     response::Response,
     router::{Builder as RouterBuilder, Router, RouterChain, RouterChainEnd},
 };
-use http::{HeaderValue, Request as RawRequest, Response as RawResponse};
+use http::{HeaderValue, Request as RawRequest, Response as RawResponse, StatusCode};
 
 /// Default time for request handling is 30 seconds
 pub const DEFAULT_REQUEST_TIMEOUT_MS: u64 = 30_000;
@@ -416,8 +416,17 @@ impl Stack {
         StackHandler { stack: self, peer_addr }
     }
 
-    async fn invoke(&self, req: Request<Body>) -> Result<Response<Body>, SaphirError> {
-        let ctx = HttpContext::new(req, self.router.clone());
+    async fn invoke(&self, mut req: Request<Body>) -> Result<Response<Body>, SaphirError> {
+        let meta = match self.router.resolve_metadata(&mut req) {
+            Ok(m) => m,
+            Err(e) => {
+                let mut r = Response::new(Body::empty());
+                *r.status_mut() = StatusCode::from_u16(e).unwrap_or(StatusCode::INTERNAL_SERVER_ERROR);
+                return Ok(r);
+            }
+        };
+        let ctx = HttpContext::new(req, self.router.clone(), meta.clone());
+
         self.middlewares
             .next(ctx)
             .await