Refactor pipeline call impl (#219)

* Refactor pipeline call impl

ntex-service/CHANGES.md

@@ -1,5 +1,9 @@
 # Changes
 
+## [1.2.3] - 2023-08-10
+
+* Check readiness for pipeline calls
+
 ## [1.2.2] - 2023-06-24
 
 * Added `ServiceCall::advance_to_call`

ntex-service/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ntex-service"
-version = "1.2.2"
+version = "1.2.3"
 authors = ["ntex contributors <team@ntex.rs>"]
 description = "ntex service"
 keywords = ["network", "framework", "async", "futures"]

ntex-service/src/apply.rs

@@ -56,7 +56,7 @@ impl<S> ApplyService<S> {
     where
         S: Service<R>,
     {
-        self.service.service_call(req)
+        self.service.call(req)
     }
 }
 
@@ -85,7 +85,6 @@ where
     type Error = Err;
     type Future<'f> = R where Self: 'f, In: 'f, R: 'f;
 
-    crate::forward_poll_ready!(service);
     crate::forward_poll_shutdown!(service);
 
     #[inline]

ntex-service/src/ctx.rs

@@ -1,6 +1,6 @@
 use std::{cell::UnsafeCell, future::Future, marker, pin::Pin, rc::Rc, task};
 
-use crate::Service;
+use crate::{Pipeline, Service};
 
 pub struct ServiceCtx<'a, S: ?Sized> {
     idx: usize,
@@ -82,7 +82,7 @@ impl Drop for Waiters {
     }
 }
 
-impl<'a, S: ?Sized> ServiceCtx<'a, S> {
+impl<'a, S> ServiceCtx<'a, S> {
     pub(crate) fn new(waiters: &'a Waiters) -> Self {
         Self {
             idx: waiters.index,
@@ -107,7 +107,7 @@ impl<'a, S: ?Sized> ServiceCtx<'a, S> {
     /// Wait for service readiness and then call service
     pub fn call<T, R>(&self, svc: &'a T, req: R) -> ServiceCall<'a, T, R>
     where
-        T: Service<R> + ?Sized,
+        T: Service<R>,
         R: 'a,
     {
         ServiceCall {
@@ -125,7 +125,7 @@ impl<'a, S: ?Sized> ServiceCtx<'a, S> {
     /// Call service, do not check service readiness
     pub fn call_nowait<T, R>(&self, svc: &'a T, req: R) -> T::Future<'a>
     where
-        T: Service<R> + ?Sized,
+        T: Service<R>,
         R: 'a,
     {
         svc.call(
@@ -139,9 +139,9 @@ impl<'a, S: ?Sized> ServiceCtx<'a, S> {
     }
 }
 
-impl<'a, S: ?Sized> Copy for ServiceCtx<'a, S> {}
+impl<'a, S> Copy for ServiceCtx<'a, S> {}
 
-impl<'a, S: ?Sized> Clone for ServiceCtx<'a, S> {
+impl<'a, S> Clone for ServiceCtx<'a, S> {
     #[inline]
     fn clone(&self) -> Self {
         Self {
@@ -157,8 +157,6 @@ pin_project_lite::pin_project! {
     pub struct ServiceCall<'a, S, Req>
     where
         S: Service<Req>,
-        S: 'a,
-        S: ?Sized,
         Req: 'a,
     {
         #[pin]
@@ -166,16 +164,45 @@ pin_project_lite::pin_project! {
     }
 }
 
+pin_project_lite::pin_project! {
+    #[project = ServiceCallStateProject]
+    enum ServiceCallState<'a, S, Req>
+    where
+        S: Service<Req>,
+        Req: 'a,
+    {
+        Ready { req: Option<Req>,
+                svc: &'a S,
+                idx: usize,
+                waiters: &'a WaitersRef,
+        },
+        ReadyPl { req: Option<Req>,
+                  svc: &'a Pipeline<S>,
+                  pl: Pipeline<S>,
+        },
+        Call { #[pin] fut: S::Future<'a> },
+        Empty,
+    }
+}
+
 impl<'a, S, Req> ServiceCall<'a, S, Req>
 where
     S: Service<Req>,
-    S: 'a,
-    S: ?Sized,
     Req: 'a,
 {
+    pub(crate) fn call_pipeline(req: Req, svc: &'a Pipeline<S>) -> Self {
+        ServiceCall {
+            state: ServiceCallState::ReadyPl {
+                req: Some(req),
+                pl: svc.clone(),
+                svc,
+            },
+        }
+    }
+
     pub fn advance_to_call(self) -> ServiceCallToCall<'a, S, Req> {
         match self.state {
-            ServiceCallState::Ready { .. } => {}
+            ServiceCallState::Ready { .. } | ServiceCallState::ReadyPl { .. } => {}
             ServiceCallState::Call { .. } | ServiceCallState::Empty => {
                 panic!(
                     "`ServiceCall::advance_to_call` must be called before `ServiceCall::poll`"
@@ -186,28 +213,9 @@ where
     }
 }
 
-pin_project_lite::pin_project! {
-    #[project = ServiceCallStateProject]
-    enum ServiceCallState<'a, S, Req>
-    where
-        S: Service<Req>,
-        S: 'a,
-        S: ?Sized,
-        Req: 'a,
-    {
-        Ready { req: Option<Req>,
-                svc: &'a S,
-                idx: usize,
-                waiters: &'a WaitersRef,
-        },
-        Call { #[pin] fut: S::Future<'a> },
-        Empty,
-    }
-}
-
 impl<'a, S, Req> Future for ServiceCall<'a, S, Req>
 where
-    S: Service<Req> + ?Sized,
+    S: Service<Req>,
 {
     type Output = Result<S::Response, S::Error>;
 
@@ -243,7 +251,21 @@ where
                     task::Poll::Pending
                 }
             },
-            ServiceCallStateProject::Call { fut } => fut.poll(cx).map(|r| {
+            ServiceCallStateProject::ReadyPl { req, svc, pl } => {
+                task::ready!(pl.poll_ready(cx))?;
+
+                let ctx = ServiceCtx::new(&svc.waiters);
+                let svc_call = svc.get_ref().call(req.take().unwrap(), ctx);
+
+                // SAFETY: `svc_call` has same lifetime same as lifetime of `pl.svc`
+                // Pipeline::svc is heap allocated(Rc<S>), we keep it alive until
+                // `svc_call` get resolved to result
+                let fut = unsafe { std::mem::transmute(svc_call) };
+
+                this.state.set(ServiceCallState::Call { fut });
+                self.poll(cx)
+            }
+            ServiceCallStateProject::Call { fut, .. } => fut.poll(cx).map(|r| {
                 this.state.set(ServiceCallState::Empty);
                 r
             }),
@@ -259,8 +281,6 @@ pin_project_lite::pin_project! {
     pub struct ServiceCallToCall<'a, S, Req>
     where
         S: Service<Req>,
-        S: 'a,
-        S: ?Sized,
         Req: 'a,
     {
         #[pin]
@@ -270,7 +290,7 @@ pin_project_lite::pin_project! {
 
 impl<'a, S, Req> Future for ServiceCallToCall<'a, S, Req>
 where
-    S: Service<Req> + ?Sized,
+    S: Service<Req>,
 {
     type Output = Result<S::Future<'a>, S::Error>;
 
@@ -306,6 +326,12 @@ where
                     task::Poll::Pending
                 }
             },
+            ServiceCallStateProject::ReadyPl { req, svc, pl } => {
+                task::ready!(pl.poll_ready(cx))?;
+
+                let ctx = ServiceCtx::new(&svc.waiters);
+                task::Poll::Ready(Ok(svc.get_ref().call(req.take().unwrap(), ctx)))
+            }
             ServiceCallStateProject::Call { .. } => {
                 unreachable!("`ServiceCallToCall` can only be constructed in `Ready` state")
             }
@@ -387,13 +413,13 @@ mod tests {
         let data1 = data.clone();
         ntex::rt::spawn(async move {
             let _ = poll_fn(|cx| srv1.poll_ready(cx)).await;
-            let i = srv1.call("srv1").await.unwrap();
+            let i = srv1.call_nowait("srv1").await.unwrap();
             data1.borrow_mut().push(i);
         });
 
         let data2 = data.clone();
         ntex::rt::spawn(async move {
-            let i = srv2.service_call("srv2").await.unwrap();
+            let i = srv2.call_static("srv2").await.unwrap();
             data2.borrow_mut().push(i);
         });
         time::sleep(time::Millis(50)).await;
@@ -417,7 +443,7 @@ mod tests {
         let con = condition::Condition::new();
         let srv = Pipeline::from(Srv(cnt.clone(), con.wait()));
 
-        let mut fut = srv.service_call("test").advance_to_call();
+        let mut fut = srv.call("test").advance_to_call();
         let _ = lazy(|cx| Pin::new(&mut fut).poll(cx)).await;
         con.notify();
 
@@ -432,7 +458,7 @@ mod tests {
         let con = condition::Condition::new();
         let srv = Pipeline::from(Srv(cnt.clone(), con.wait()));
 
-        let mut fut = srv.service_call("test");
+        let mut fut = srv.call("test");
         let _ = lazy(|cx| Pin::new(&mut fut).poll(cx)).await;
         con.notify();
 

ntex-service/src/fn_shutdown.rs

@@ -85,7 +85,7 @@ mod tests {
 
         let pipe = Pipeline::new(chain(srv).and_then(on_shutdown).clone());
 
-        let res = pipe.service_call(()).await;
+        let res = pipe.call(()).await;
         assert_eq!(lazy(|cx| pipe.poll_ready(cx)).await, Poll::Ready(Ok(())));
         assert!(res.is_ok());
         assert_eq!(res.unwrap(), "pipe");

ntex-service/src/lib.rs

@@ -261,7 +261,7 @@ pub trait ServiceFactory<Req, Cfg = ()> {
 
 impl<'a, S, Req> Service<Req> for &'a S
 where
-    S: Service<Req> + ?Sized,
+    S: Service<Req>,
 {
     type Response = S::Response;
     type Error = S::Error;
@@ -285,7 +285,7 @@ where
 
 impl<S, Req> Service<Req> for Box<S>
 where
-    S: Service<Req> + ?Sized,
+    S: Service<Req>,
 {
     type Response = S::Response;
     type Error = S::Error;

ntex-service/src/pipeline.rs

@@ -1,15 +1,14 @@
-use std::{cell::Cell, future, pin::Pin, rc::Rc, task::Context, task::Poll};
+use std::{cell::Cell, future, pin::Pin, rc::Rc, task, task::Context, task::Poll};
 
-use crate::ctx::{ServiceCall, ServiceCtx, Waiters};
-use crate::{Service, ServiceFactory};
+use crate::{ctx::ServiceCall, ctx::Waiters, Service, ServiceCtx, ServiceFactory};
 
 /// Container for a service.
 ///
 /// Container allows to call enclosed service and adds support of shared readiness.
 pub struct Pipeline<S> {
     svc: Rc<S>,
-    waiters: Waiters,
     pending: Cell<bool>,
+    pub(crate) waiters: Waiters,
 }
 
 impl<S> Pipeline<S> {
@@ -55,33 +54,53 @@ impl<S> Pipeline<S> {
         self.svc.poll_shutdown(cx)
     }
 
+    #[deprecated(since = "1.2.3", note = "Use Pipeline::call() instead")]
+    #[doc(hidden)]
     #[inline]
     /// Wait for service readiness and then create future object
     /// that resolves to service result.
-    pub fn service_call<'a, R>(&'a self, req: R) -> ServiceCall<'a, S, R>
+    pub fn service_call<R>(&self, req: R) -> ServiceCall<'_, S, R>
     where
         S: Service<R>,
     {
-        ServiceCtx::<'a, S>::new(&self.waiters).call(self.svc.as_ref(), req)
+        ServiceCall::call_pipeline(req, self)
+    }
+
+    #[inline]
+    /// Wait for service readiness and then create future object
+    /// that resolves to service result.
+    pub fn call<R>(&self, req: R) -> ServiceCall<'_, S, R>
+    where
+        S: Service<R>,
+    {
+        ServiceCall::call_pipeline(req, self)
+    }
+
+    #[inline]
+    /// Wait for service readiness and then create future object
+    /// that resolves to service result.
+    pub fn call_static<R>(&self, req: R) -> PipelineCall<S, R>
+    where
+        S: Service<R> + 'static,
+    {
+        PipelineCall {
+            state: PipelineCallState::Ready { req: Some(req) },
+            pipeline: self.clone(),
+        }
     }
 
     #[inline]
     /// Call service and create future object that resolves to service result.
     ///
     /// Note, this call does not check service readiness.
-    pub fn call<R>(&self, req: R) -> PipelineCall<S, R>
+    pub fn call_nowait<R>(&self, req: R) -> PipelineCall<S, R>
     where
         S: Service<R> + 'static,
-        R: 'static,
     {
-        let pipeline = self.clone();
-        let svc_call = pipeline.svc.call(req, ServiceCtx::new(&pipeline.waiters));
-
-        // SAFETY: `svc_call` has same lifetime same as lifetime of `pipeline.svc`
-        // Pipeline::svc is heap allocated(Rc<S>), we keep it alive until
-        // `svc_call` get resolved to result
-        let fut = unsafe { std::mem::transmute(svc_call) };
-        PipelineCall { fut, pipeline }
+        PipelineCall {
+            state: PipelineCallState::new_call(self, req),
+            pipeline: self.clone(),
+        }
     }
 
     /// Extract service if container hadnt been cloned before.
@@ -119,11 +138,43 @@ pin_project_lite::pin_project! {
         R: 'static,
     {
         #[pin]
-        fut: S::Future<'static>,
+        state: PipelineCallState<S, R>,
         pipeline: Pipeline<S>,
     }
 }
 
+pin_project_lite::pin_project! {
+    #[project = PipelineCallStateProject]
+    enum PipelineCallState<S, Req>
+    where
+        S: Service<Req>,
+        S: 'static,
+        Req: 'static,
+    {
+        Ready { req: Option<Req> },
+        Call { #[pin] fut: S::Future<'static> },
+        Empty,
+    }
+}
+
+impl<S, R> PipelineCallState<S, R>
+where
+    S: Service<R> + 'static,
+    R: 'static,
+{
+    fn new_call(pl: &Pipeline<S>, req: R) -> Self {
+        let ctx = ServiceCtx::new(&pl.waiters);
+        let svc_call = pl.get_ref().call(req, ctx);
+
+        // SAFETY: `svc_call` has same lifetime same as lifetime of `pl.svc`
+        // Pipeline::svc is heap allocated(Rc<S>), we keep it alive until
+        // `svc_call` get resolved to result
+        let fut = unsafe { std::mem::transmute(svc_call) };
+
+        PipelineCallState::Call { fut }
+    }
+}
+
 impl<S, R> future::Future for PipelineCall<S, R>
 where
     S: Service<R>,
@@ -131,8 +182,25 @@ where
     type Output = Result<S::Response, S::Error>;
 
     #[inline]
-    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        self.project().fut.poll(cx)
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        let mut this = self.as_mut().project();
+
+        match this.state.as_mut().project() {
+            PipelineCallStateProject::Ready { req } => {
+                task::ready!(this.pipeline.poll_ready(cx))?;
+
+                let st = PipelineCallState::new_call(this.pipeline, req.take().unwrap());
+                this.state.set(st);
+                self.poll(cx)
+            }
+            PipelineCallStateProject::Call { fut, .. } => fut.poll(cx).map(|r| {
+                this.state.set(PipelineCallState::Empty);
+                r
+            }),
+            PipelineCallStateProject::Empty => {
+                panic!("future must not be polled after it returned `Poll::Ready`")
+            }
+        }
     }
 }