util: Add then combinator
#500
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, the `ServiceExt` trait has `with` and `try_with` methods for composing a `Service` with functions that modify the request type, and `map_err` and `map_ok` methods for composing a service with functions that modify the response type. Meanwhile, `ServiceBuilder` has `map_request` for composing a service with a request mapping function, and `map_response` for composing a service with a response-mapping function. These combinators are very similar in purpose, but are implemented using different middleware types that essentially duplicate the same behavior. Before releasing 0.4, we should probably unify these APIs. In particular, it would be good to de-duplicate the middleware service types, and to unify the naming. This commit makes the following changes: - Rename the `ServiceExt::with` and `ServiceExt::try_with` combinators to `map_request` and `try_map_request` - Rename the `ServiceExt::map_ok` combinator to `map_response` - Unify the `ServiceBuilder::map_request` and `ServiceExt::map_request` combinators to use the same `Service` type - Unify the `ServiceBuilder::map_response` and `ServiceExt::map_response` combinators to use the same `Service` type - Unify the `ServiceBuilder::map_err` and `ServiceExt::map_err` combinators to use the same `Service` type - Only take `FnOnce + Clone` when in response/err combinators, which require cloning into the future type. `MapRequest` and `TryMapRequest` now take `FnMut(Request)`s and don't clone them every time they're called - Reexport future types for combinators where it makes sense. - Add a `try_map_request` method to `ServiceBuilder` Closes #498
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Currently, `ServiceExt` and `ServiceBuilder` provide combinators for mapping successful responses to other responses, and mapping errors to other errors, but don't provide a way to map between `Ok` and `Err` results. For completeness, this branch adds a new `then` combinator, which takes a function from `Result` to `Result` and applies it when the service's future completes. This can be used for recovering from some errors or for rejecting some `Ok` responses. It can also be used for behaviors that should be run when a service's future completes regardless of whether it completed successfully or not. Depends on #499
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw
force-pushed
the
eliza/then-combinator
branch
from
d2a0f29
to
1f17c94
Compare
|
This seems somewhat incongruous with What we have here seems to be more like |
This is a good point --- I got the naming mixed up. |
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
…nto eliza/unify-combinators
Currently, `ServiceExt` and `ServiceBuilder` provide combinators for mapping successful responses to other responses, and mapping errors to other errors, but don't provide a way to map between `Ok` and `Err` results. For completeness, this branch adds a new `then` combinator, which takes a function from `Result` to `Result` and applies it when the service's future completes. This can be used for recovering from some errors or for rejecting some `Ok` responses. It can also be used for behaviors that should be run when a service's future completes regardless of whether it completed successfully or not. Depends on #499
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
LucioFranco
approved these changes
Jan 4, 2021
tower/src/util/mod.rs
Outdated
| @@ -327,6 +542,17 @@ pub trait ServiceExt<Request>: tower_service::Service<Request> { | |||
| { | |||
| TryMapRequest::new(self, f) | |||
| } | |||
|
|
|||
| /// TODO(eliza): docs | |||
…o eliza/then-combinator Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently,
ServiceExtandServiceBuilderprovide combinators formapping successful responses to other responses, and mapping errors to
other errors, but don't provide a way to map between
OkandErrresults.
For completeness, this branch adds a new
thencombinator, which takesa function from
ResulttoResultand applies it when the service'sfuture completes. This can be used for recovering from some errors or
for rejecting some
Okresponses. It can also be used for behaviorsthat should be run when a service's future completes regardless of
whether it completed successfully or not.
Depends on #499