util: Add ReadyAnd to do what Ready should do (#427)

* util: Add ReadyAnd to do what Ready should do

`ServiceExt::ready` says that it produces "A future yielding the service
when it is ready to accept a request." This is not true; it does _not_
yield the service when it is ready, it yields unit. This makes it
impossible to chain service ready with service call, which is sad.

This PR adds `ready_and`, which does what `ready` promised. It also
deprecates `ready` with the intention that we remove `ready` in a future
version, and make the strictly more general `ready_and` take its place.
We can't do it now since it's not a backwards-compatible change even
though it _probably_ wouldn't break any code.

The PR also updates the docs so that they reflect the observed behavior.

tower-util/src/lib.rs

@@ -23,7 +23,7 @@ pub use crate::{
     either::Either,
     oneshot::Oneshot,
     optional::Optional,
-    ready::Ready,
+    ready::{Ready, ReadyAnd, ReadyOneshot},
     service_fn::{service_fn, ServiceFn},
 };
 
@@ -48,14 +48,32 @@ pub mod future {
 /// An extension trait for `Service`s that provides a variety of convenient
 /// adapters
 pub trait ServiceExt<Request>: tower_service::Service<Request> {
-    /// A future yielding the service when it is ready to accept a request.
+    /// Resolves when the service is ready to accept a request.
+    #[deprecated(since = "0.3.1", note = "prefer `ready_and` which yields the service")]
     fn ready(&mut self) -> Ready<'_, Self, Request>
     where
         Self: Sized,
     {
+        #[allow(deprecated)]
         Ready::new(self)
     }
 
+    /// Yields a mutable reference to the service when it is ready to accept a request.
+    fn ready_and(&mut self) -> ReadyAnd<'_, Self, Request>
+    where
+        Self: Sized,
+    {
+        ReadyAnd::new(self)
+    }
+
+    /// Yields the service when it is ready to accept a request.
+    fn ready_oneshot(self) -> ReadyOneshot<Self, Request>
+    where
+        Self: Sized,
+    {
+        ReadyOneshot::new(self)
+    }
+
     /// Consume this `Service`, calling with the providing request once it is ready.
     fn oneshot(self, req: Request) -> Oneshot<Self, Request>
     where

tower-util/src/ready.rs

@@ -8,15 +8,104 @@ use std::{
 };
 use tower_service::Service;
 
-/// Future yielding a `Service` once the service is ready to process a request
+/// A future that yields the service when it is ready to accept a request.
 ///
-/// `Ready` values are produced by `ServiceExt::ready`.
-pub struct Ready<'a, T, Request> {
-    inner: &'a mut T,
+/// `ReadyOneshot` values are produced by `ServiceExt::ready_oneshot`.
+pub struct ReadyOneshot<T, Request> {
+    inner: Option<T>,
     _p: PhantomData<fn() -> Request>,
 }
 
 // Safety: This is safe because `Services`'s are always `Unpin`.
+impl<T, Request> Unpin for ReadyOneshot<T, Request> {}
+
+impl<T, Request> ReadyOneshot<T, Request>
+where
+    T: Service<Request>,
+{
+    #[allow(missing_docs)]
+    pub fn new(service: T) -> Self {
+        Self {
+            inner: Some(service),
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<T, Request> Future for ReadyOneshot<T, Request>
+where
+    T: Service<Request>,
+{
+    type Output = Result<T, T::Error>;
+
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        ready!(self
+            .inner
+            .as_mut()
+            .expect("poll after Poll::Ready")
+            .poll_ready(cx))?;
+
+        Poll::Ready(Ok(self.inner.take().expect("poll after Poll::Ready")))
+    }
+}
+
+impl<T, Request> fmt::Debug for ReadyOneshot<T, Request>
+where
+    T: fmt::Debug,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("ReadyOneshot")
+            .field("inner", &self.inner)
+            .finish()
+    }
+}
+
+/// A future that yields a mutable reference to the service when it is ready to accept a request.
+///
+/// `ReadyAnd` values are produced by `ServiceExt::ready_and`.
+pub struct ReadyAnd<'a, T, Request>(ReadyOneshot<&'a mut T, Request>);
+
+// Safety: This is safe for the same reason that the impl for ReadyOneshot is safe.
+impl<'a, T, Request> Unpin for ReadyAnd<'a, T, Request> {}
+
+impl<'a, T, Request> ReadyAnd<'a, T, Request>
+where
+    T: Service<Request>,
+{
+    #[allow(missing_docs)]
+    pub fn new(service: &'a mut T) -> Self {
+        Self(ReadyOneshot::new(service))
+    }
+}
+
+impl<'a, T, Request> Future for ReadyAnd<'a, T, Request>
+where
+    T: Service<Request>,
+{
+    type Output = Result<&'a mut T, T::Error>;
+
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        Pin::new(&mut self.0).poll(cx)
+    }
+}
+
+impl<'a, T, Request> fmt::Debug for ReadyAnd<'a, T, Request>
+where
+    T: fmt::Debug,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("ReadyAnd").field(&self.0).finish()
+    }
+}
+
+// ==== deprecated `Ready` that is just `ReadyAnd` with a unit return type.
+
+/// A future that resolves when a `Service` is ready to accept a request.
+///
+/// `Ready` values are produced by `ServiceExt::ready`.
+pub struct Ready<'a, T, Request>(ReadyAnd<'a, T, Request>);
+
+// Safety: This is safe for the same reason that the impl for ReadyOneshot is safe.
 impl<'a, T, Request> Unpin for Ready<'a, T, Request> {}
 
 impl<'a, T, Request> Ready<'a, T, Request>
@@ -24,11 +113,9 @@ where
     T: Service<Request>,
 {
     #[allow(missing_docs)]
+    #[deprecated(since = "0.3.1", note = "prefer `ReadyAnd` which yields the service")]
     pub fn new(service: &'a mut T) -> Self {
-        Ready {
-            inner: service,
-            _p: PhantomData,
-        }
+        Self(ReadyAnd::new(service))
     }
 }
 
@@ -39,8 +126,7 @@ where
     type Output = Result<(), T::Error>;
 
     fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        ready!(self.inner.poll_ready(cx))?;
-
+        let _ = ready!(Pin::new(&mut self.0).poll(cx));
         Poll::Ready(Ok(()))
     }
 }
@@ -50,6 +136,6 @@ where
     T: fmt::Debug,
 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("Ready").field("inner", &self.inner).finish()
+        f.debug_tuple("Ready").field(&self.0).finish()
     }
 }

tower/tests/builder.rs

@@ -25,8 +25,7 @@ async fn builder_service() {
     // allow a request through
     handle.allow(1);
 
-    client.ready().await.unwrap();
-    let fut = client.call("hello");
+    let fut = client.ready_and().await.unwrap().call("hello");
     assert_request_eq!(handle, "hello").send_response("world");
     assert_eq!(fut.await.unwrap(), "world");
 }