Skip to content

Commit

Permalink
util: Rename ServiceExt::ready_and to ServiceExt::ready (#567)
Browse files Browse the repository at this point in the history
This PR renames: 

- `ServiceExt::ready_and` to `ServiceExt::ready`
- the `ReadyAnd` future to `Ready`
- the associated documentation to refer to `ServiceExt::ready`
  and `ReadyAnd`.

This PR deprecates:

- the `ServiceExt::ready_and` method
- the `ReadyAnd` future

These can be removed in Tower 0.5.

My recollection of the original conversation surrounding the
introduction of the `ServiceExt::ready_and` combinator in
#427 was that it was meant to be a
temporary workaround for the unchainable `ServiceExt::ready` combinator
until the next breaking release of the Tower crate. The unchainable
`ServiceExt::ready` combinator was removed, but `ServiceExt::ready_and`
was not renamed. I believe, but am not 100% sure, that this was an
oversight.
  • Loading branch information
davidbarsky authored Feb 25, 2021
1 parent 04369f3 commit a8884ae
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 59 deletions.
2 changes: 1 addition & 1 deletion tower/src/buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
//! let mut svc = svc.clone();
//! tokio::spawn(async move {
//! for i in 0usize.. {
//! svc.ready_and().await.expect("service crashed").call(i).await;
//! svc.ready().await.expect("service crashed").call(i).await;
//! }
//! });
//! }
Expand Down
4 changes: 2 additions & 2 deletions tower/src/load/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@
//! S2: Load<Metric = S1::Metric> + Service<R, Response = S1::Response, Error = S1::Error>
//! {
//! if svc1.load() < svc2.load() {
//! svc1.ready_and().await?.call(request).await
//! svc1.ready().await?.call(request).await
//! } else {
//! svc2.ready_and().await?.call(request).await
//! svc2.ready().await?.call(request).await
//! }
//! }
//! ```
Expand Down
4 changes: 2 additions & 2 deletions tower/src/steer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@
//!
//! // This request will get sent to `root`
//! let req = Request::get("/").body(String::new()).unwrap();
//! let res = svc.ready_and().await?.call(req).await?;
//! let res = svc.ready().await?.call(req).await?;
//! assert_eq!(res.into_body(), "Hello, World!");
//!
//! // This request will get sent to `not_found`
//! let req = Request::get("/does/not/exist").body(String::new()).unwrap();
//! let res = svc.ready_and().await?.call(req).await?;
//! let res = svc.ready().await?.call(req).await?;
//! assert_eq!(res.status(), StatusCode::NOT_FOUND);
//! assert_eq!(res.into_body(), "");
//! #
Expand Down
6 changes: 3 additions & 3 deletions tower/src/util/future_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use tower_service::Service;
///
/// // Now, when we wait for the service to become ready, it will
/// // drive the future to completion internally.
/// let svc = svc.ready_and().await.unwrap();
/// let svc = svc.ready().await.unwrap();
/// let res = svc.call(()).await.unwrap();
/// # };
/// # }
Expand Down Expand Up @@ -87,7 +87,7 @@ impl<F, S> FutureService<F, S> {
///
/// // Now, when we wait for the service to become ready, it will
/// // drive the future to completion internally.
/// let svc = svc.ready_and().await.unwrap();
/// let svc = svc.ready().await.unwrap();
/// let res = svc.call(()).await.unwrap();
/// # };
/// # }
Expand Down Expand Up @@ -188,7 +188,7 @@ mod tests {
"FutureService { state: State::Future(<futures_util::future::ready::Ready<core::result::Result<tower::util::future_service::tests::DebugService, core::convert::Infallible>>>) }"
);

pending_svc.ready_and().await.unwrap();
pending_svc.ready().await.unwrap();

assert_eq!(
format!("{:?}", pending_svc),
Expand Down
36 changes: 25 additions & 11 deletions tower/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod ready;
mod service_fn;
mod then;

#[allow(deprecated)]
pub use self::{
and_then::{AndThen, AndThenLayer},
boxed::{BoxService, UnsyncBoxService},
Expand All @@ -30,7 +31,7 @@ pub use self::{
map_result::{MapResult, MapResultLayer},
oneshot::Oneshot,
optional::Optional,
ready::{ReadyAnd, ReadyOneshot},
ready::{Ready, ReadyAnd, ReadyOneshot},
service_fn::{service_fn, ServiceFn},
then::{Then, ThenLayer},
};
Expand Down Expand Up @@ -61,6 +62,19 @@ pub mod future {
/// adapters
pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// Yields a mutable reference to the service when it is ready to accept a request.
fn ready(&mut self) -> Ready<'_, Self, Request>
where
Self: Sized,
{
Ready::new(self)
}

/// Yields a mutable reference to the service when it is ready to accept a request.
#[deprecated(
since = "0.4.6",
note = "please use the `ServiceExt::ready` method instead"
)]
#[allow(deprecated)]
fn ready_and(&mut self) -> ReadyAnd<'_, Self, Request>
where
Self: Sized,
Expand Down Expand Up @@ -222,7 +236,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let name = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await?;
Expand Down Expand Up @@ -289,7 +303,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let code = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await
Expand Down Expand Up @@ -390,7 +404,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let name = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await?;
Expand Down Expand Up @@ -460,7 +474,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let record = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await?;
Expand Down Expand Up @@ -511,7 +525,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let response = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await;
Expand Down Expand Up @@ -579,7 +593,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let response = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await;
Expand Down Expand Up @@ -648,7 +662,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = "13";
/// let response = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await;
Expand Down Expand Up @@ -735,7 +749,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// let id = 13;
/// # let id: u32 = id;
/// let response = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await;
Expand Down Expand Up @@ -837,7 +851,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let record = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await?;
Expand Down Expand Up @@ -915,7 +929,7 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> {
/// // Call the new service
/// let id = 13;
/// let record = new_service
/// .ready_and()
/// .ready()
/// .await?
/// .call(id)
/// .await?;
Expand Down
20 changes: 14 additions & 6 deletions tower/src/util/ready.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,25 @@ where
}
}

/// A future that yields a mutable reference to the service when it is ready to accept a request.
///
/// [`Ready`] values are produced by [`ServiceExt::ready`].
///
/// [`ServiceExt::ready`]: crate::util::ServiceExt::ready
pub struct Ready<'a, T, Request>(ReadyOneshot<&'a mut T, Request>);

/// 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`].
///
/// [`ServiceExt::ready_and`]: crate::util::ServiceExt::ready_and
pub struct ReadyAnd<'a, T, Request>(ReadyOneshot<&'a mut T, Request>);
#[deprecated(since = "0.4.6", note = "Please use the Ready future instead")]
pub type ReadyAnd<'a, T, Request> = Ready<'a, 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> Unpin for Ready<'a, T, Request> {}

impl<'a, T, Request> ReadyAnd<'a, T, Request>
impl<'a, T, Request> Ready<'a, T, Request>
where
T: Service<Request>,
{
Expand All @@ -82,7 +90,7 @@ where
}
}

impl<'a, T, Request> Future for ReadyAnd<'a, T, Request>
impl<'a, T, Request> Future for Ready<'a, T, Request>
where
T: Service<Request>,
{
Expand All @@ -93,11 +101,11 @@ where
}
}

impl<'a, T, Request> fmt::Debug for ReadyAnd<'a, T, Request>
impl<'a, T, Request> fmt::Debug for Ready<'a, T, Request>
where
T: fmt::Debug,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("ReadyAnd").field(&self.0).finish()
f.debug_tuple("Ready").field(&self.0).finish()
}
}
66 changes: 33 additions & 33 deletions tower/tests/buffer/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,19 +239,19 @@ async fn wakes_pending_waiters_on_close() {

// keep the request in the worker
handle.allow(0);
let service1 = service.ready_and().await.unwrap();
let service1 = service.ready().await.unwrap();
assert_pending!(worker.poll());
let mut response = task::spawn(service1.call("hello"));

let mut service1 = service.clone();
let mut ready_and1 = task::spawn(service1.ready_and());
let mut ready1 = task::spawn(service1.ready());
assert_pending!(worker.poll());
assert_pending!(ready_and1.poll(), "no capacity");
assert_pending!(ready1.poll(), "no capacity");

let mut service1 = service.clone();
let mut ready_and2 = task::spawn(service1.ready_and());
let mut ready2 = task::spawn(service1.ready());
assert_pending!(worker.poll());
assert_pending!(ready_and2.poll(), "no capacity");
assert_pending!(ready2.poll(), "no capacity");

// kill the worker task
drop(worker);
Expand All @@ -264,24 +264,24 @@ async fn wakes_pending_waiters_on_close() {
);

assert!(
ready_and1.is_woken(),
"dropping worker should wake ready_and task 1"
ready1.is_woken(),
"dropping worker should wake ready task 1"
);
let err = assert_ready_err!(ready_and1.poll());
let err = assert_ready_err!(ready1.poll());
assert!(
err.is::<error::Closed>(),
"ready_and 1 should fail with a Closed, got: {:?}",
"ready 1 should fail with a Closed, got: {:?}",
err
);

assert!(
ready_and2.is_woken(),
"dropping worker should wake ready_and task 2"
ready2.is_woken(),
"dropping worker should wake ready task 2"
);
let err = assert_ready_err!(ready_and1.poll());
let err = assert_ready_err!(ready1.poll());
assert!(
err.is::<error::Closed>(),
"ready_and 2 should fail with a Closed, got: {:?}",
"ready 2 should fail with a Closed, got: {:?}",
err
);
}
Expand All @@ -297,19 +297,19 @@ async fn wakes_pending_waiters_on_failure() {

// keep the request in the worker
handle.allow(0);
let service1 = service.ready_and().await.unwrap();
let service1 = service.ready().await.unwrap();
assert_pending!(worker.poll());
let mut response = task::spawn(service1.call("hello"));

let mut service1 = service.clone();
let mut ready_and1 = task::spawn(service1.ready_and());
let mut ready1 = task::spawn(service1.ready());
assert_pending!(worker.poll());
assert_pending!(ready_and1.poll(), "no capacity");
assert_pending!(ready1.poll(), "no capacity");

let mut service1 = service.clone();
let mut ready_and2 = task::spawn(service1.ready_and());
let mut ready2 = task::spawn(service1.ready());
assert_pending!(worker.poll());
assert_pending!(ready_and2.poll(), "no capacity");
assert_pending!(ready2.poll(), "no capacity");

// fail the inner service
handle.send_error("foobar");
Expand All @@ -324,24 +324,24 @@ async fn wakes_pending_waiters_on_failure() {
);

assert!(
ready_and1.is_woken(),
"dropping worker should wake ready_and task 1"
ready1.is_woken(),
"dropping worker should wake ready task 1"
);
let err = assert_ready_err!(ready_and1.poll());
let err = assert_ready_err!(ready1.poll());
assert!(
err.is::<error::ServiceError>(),
"ready_and 1 should fail with a ServiceError, got: {:?}",
"ready 1 should fail with a ServiceError, got: {:?}",
err
);

assert!(
ready_and2.is_woken(),
"dropping worker should wake ready_and task 2"
ready2.is_woken(),
"dropping worker should wake ready task 2"
);
let err = assert_ready_err!(ready_and1.poll());
let err = assert_ready_err!(ready1.poll());
assert!(
err.is::<error::ServiceError>(),
"ready_and 2 should fail with a ServiceError, got: {:?}",
"ready 2 should fail with a ServiceError, got: {:?}",
err
);
}
Expand Down Expand Up @@ -379,19 +379,19 @@ async fn doesnt_leak_permits() {
// Attempt to poll the first clone of the buffer to readiness multiple
// times. These should all succeed, because the readiness is never
// *consumed* --- no request is sent.
assert_ready_ok!(task::spawn(service1.ready_and()).poll());
assert_ready_ok!(task::spawn(service1.ready_and()).poll());
assert_ready_ok!(task::spawn(service1.ready_and()).poll());
assert_ready_ok!(task::spawn(service1.ready()).poll());
assert_ready_ok!(task::spawn(service1.ready()).poll());
assert_ready_ok!(task::spawn(service1.ready()).poll());

// It should also be possible to drive the second clone of the service to
// readiness --- it should only acquire one permit, as well.
assert_ready_ok!(task::spawn(service2.ready_and()).poll());
assert_ready_ok!(task::spawn(service2.ready_and()).poll());
assert_ready_ok!(task::spawn(service2.ready_and()).poll());
assert_ready_ok!(task::spawn(service2.ready()).poll());
assert_ready_ok!(task::spawn(service2.ready()).poll());
assert_ready_ok!(task::spawn(service2.ready()).poll());

// The third clone *doesn't* poll ready, because the first two clones have
// each acquired one permit.
let mut ready3 = task::spawn(service3.ready_and());
let mut ready3 = task::spawn(service3.ready());
assert_pending!(ready3.poll());

// Consume the first service's readiness.
Expand Down
2 changes: 1 addition & 1 deletion tower/tests/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async fn builder_service() {
// allow a request through
handle.allow(1);

let fut = client.ready_and().await.unwrap().call("hello");
let fut = client.ready().await.unwrap().call("hello");
assert_request_eq!(handle, true).send_response("world");
assert_eq!(fut.await.unwrap(), true);
}
Expand Down

0 comments on commit a8884ae

Please sign in to comment.