From 86fc5f25c9b0122ad9df75670a9d1dfc4c9ed8b7 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Tue, 2 Feb 2021 12:42:15 -0500 Subject: [PATCH] Add middleware & update design docs (#175) * Add middleware & update design docs * Add missing design folder * Improve MapRequest middleware example * Vendor pin_utils::pin_mut * Remove Debug body restrictions * Modify error bound * Remove redundant return --- design/src/SUMMARY.md | 1 + design/src/middleware.md | 7 ++ design/src/operation.md | 50 ++++----- rust-runtime/smithy-http/src/lib.rs | 3 + rust-runtime/smithy-http/src/middleware.rs | 123 +++++++++++++++++++++ rust-runtime/smithy-http/src/pin_util.rs | 30 +++++ rust-runtime/smithy-http/src/response.rs | 48 +++++++- rust-runtime/smithy-http/src/result.rs | 59 ++++++++++ 8 files changed, 288 insertions(+), 33 deletions(-) create mode 100644 design/src/middleware.md create mode 100644 rust-runtime/smithy-http/src/middleware.rs create mode 100644 rust-runtime/smithy-http/src/pin_util.rs create mode 100644 rust-runtime/smithy-http/src/result.rs diff --git a/design/src/SUMMARY.md b/design/src/SUMMARY.md index b0ebab3db0..0f950fa4b7 100644 --- a/design/src/SUMMARY.md +++ b/design/src/SUMMARY.md @@ -1,3 +1,4 @@ # Summary - [Http Operations](./operation.md) +- [HTTP middleware](./middleware.md) diff --git a/design/src/middleware.md b/design/src/middleware.md new file mode 100644 index 0000000000..07a1e47388 --- /dev/null +++ b/design/src/middleware.md @@ -0,0 +1,7 @@ +# HTTP middleware + +Signing, endpoint specification, and logging are all handled as middleware. The Rust SDK takes a minimalist approach to middleware: + +Middleware is defined as minimally as possible, then adapted into the middleware system used by the IO layer. Tower is the de facto standard for HTTP middleware in Rust—we will probably use it. But we also want to make our middleware usable for users who aren't using Tower (or if we decide to not use Tower in the long run). + +Because of this, rather than implementing all our middleware as "Tower Middleware", we implement it narrowly (eg. as a function that operates on `operation::Request`), then define optional adapters to make our middleware tower compatible. diff --git a/design/src/operation.md b/design/src/operation.md index fe9bb3b790..d1045a31d9 100644 --- a/design/src/operation.md +++ b/design/src/operation.md @@ -17,9 +17,10 @@ A customer interacts with the SDK builders to construct an input. The `build()` an `Operation`. This codifies the base HTTP request & all the configuration and middleware layers required to modify and dispatch the request. ```rust,ignore -pub struct Operation { +pub struct Operation { request: Request, - response_handler: Box, + response_handler: H, + _retry_policy: R, } pub struct Request { @@ -42,33 +43,22 @@ pub fn build(self, config: &dynamodb::config::Config) -> Operation; + +/// [`MapRequest`] defines a synchronous middleware that transforms an [`operation::Request`]. +/// +/// Typically, these middleware will read configuration from the `PropertyBag` and use it to +/// augment the request. Most fundamental middleware is expressed as `MapRequest`, including +/// signing & endpoint resolution. +/// +/// ```rust +/// # use smithy_http::middleware::MapRequest; +/// # use std::convert::Infallible; +/// # use smithy_http::operation; +/// use http::header::{HeaderName, HeaderValue}; +/// struct AddHeader(HeaderName, HeaderValue); +/// /// Signaling struct added to the request property bag if a header should be added +/// struct NeedsHeader; +/// impl MapRequest for AddHeader { +/// type Error = Infallible; +/// fn apply(&self, request: operation::Request) -> Result { +/// request.augment(|mut request, properties| { +/// if properties.get::().is_some() { +/// request.headers_mut().append( +/// self.0.clone(), +/// self.1.clone(), +/// ); +/// } +/// Ok(request) +/// }) +/// } +/// } +/// ``` +pub trait MapRequest { + /// The Error type returned by this operation. + /// + /// If this middleware never fails use [std::convert::Infallible] or similar. + type Error: Into; + + /// Apply this middleware to a request. + /// + /// Typically, implementations will use [`request.augment`](crate::operation::Request::augment) + /// to be able to transform an owned `http::Request`. + fn apply(&self, request: operation::Request) -> Result; +} + +/// Load a response using `handler` to parse the results. +/// +/// This function is intended to be used on the response side of a middleware chain. +/// +/// Success and failure will be split and mapped into `SdkSuccess` and `SdkError`. +/// Generic Parameters: +/// - `B`: The Response Body +/// - `O`: The Http response handler that returns `Result` +/// - `T`/`E`: `Result` returned by `handler`. +pub async fn load_response( + mut response: http::Response, + handler: &O, +) -> Result, SdkError> +where + B: http_body::Body + Unpin, + B: From + 'static, + B::Error: Into, + O: ParseHttpResponse>, +{ + if let Some(parsed_response) = handler.parse_unloaded(&mut response) { + return sdk_result(parsed_response, response); + } + + let body = match read_body(response.body_mut()).await { + Ok(body) => body, + Err(e) => { + return Err(SdkError::ResponseError { + raw: response, + err: e.into(), + }); + } + }; + + let response = response.map(|_| Bytes::from(body)); + let parsed = handler.parse_loaded(&response); + sdk_result(parsed, response.map(B::from)) +} + +async fn read_body(body: B) -> Result, B::Error> { + let mut output = Vec::new(); + pin_mut!(body); + while let Some(buf) = body.data().await { + let mut buf = buf?; + while buf.has_remaining() { + output.extend_from_slice(buf.chunk()); + buf.advance(buf.chunk().len()) + } + } + Ok(output) +} + +/// Convert a `Result` into an `SdkResult` that includes the raw HTTP response +fn sdk_result( + parsed: Result, + raw: http::Response, +) -> Result, SdkError> { + match parsed { + Ok(parsed) => Ok(SdkSuccess { raw, parsed }), + Err(err) => Err(SdkError::ServiceError { raw, err }), + } +} diff --git a/rust-runtime/smithy-http/src/pin_util.rs b/rust-runtime/smithy-http/src/pin_util.rs new file mode 100644 index 0000000000..b0dd6e7bf0 --- /dev/null +++ b/rust-runtime/smithy-http/src/pin_util.rs @@ -0,0 +1,30 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +/// Pins a value on the stack. +/// +/// # Example +/// +/// ```rust +/// # use core::pin::Pin; +/// # struct Foo {} +/// # use smithy_http::pin_mut; +/// let foo = Foo { /* ... */ }; +/// pin_mut!(foo); +/// let _: Pin<&mut Foo> = foo; +/// ``` +#[macro_export] +macro_rules! pin_mut { + ($($x:ident),* $(,)?) => { $( + // Move the value to ensure that it is owned + let mut $x = $x; + // Shadow the original binding so that it can't be directly accessed + // ever again. + #[allow(unused_mut)] + let mut $x = unsafe { + core::pin::Pin::new_unchecked(&mut $x) + }; + )* } +} diff --git a/rust-runtime/smithy-http/src/response.rs b/rust-runtime/smithy-http/src/response.rs index 79d72c1d5c..cd2ca57a4f 100644 --- a/rust-runtime/smithy-http/src/response.rs +++ b/rust-runtime/smithy-http/src/response.rs @@ -6,7 +6,7 @@ use bytes::Bytes; use http::Response; -/// `ParseHttpResponse` is a generic trait for parsing structured data from HTTP respsones. +/// `ParseHttpResponse` is a generic trait for parsing structured data from HTTP responses. /// /// It is designed to be nearly infinitely flexible, because `Output` is unconstrained, it can be used to support /// event streams, S3 streaming responses, regular request-response style operations, as well @@ -47,10 +47,16 @@ pub trait ParseHttpResponse { fn parse_unloaded(&self, response: &mut http::Response) -> Option; /// Parse an HTTP request from a fully loaded body. This is for standard request/response style - /// APIs like AwsJSON as well as for the error path of most streaming APIs + /// APIs like AwsJson 1.0/1.1 and the error path of most streaming APIs /// /// Using an explicit body type of Bytes here is a conscious decision—If you _really_ need - /// to precisely control how the data is loaded into memory, use `parse_unloaded`. + /// to precisely control how the data is loaded into memory (eg. by using `bytes::Buf`), implement + /// your handler in `parse_unloaded`. + /// + /// Production code will never call `parse_loaded` without first calling `parse_unloaded`. However, + /// in tests it may be easier to use `parse_loaded` directly. It is OK to panic in `parse_loaded` + /// if `parse_unloaded` will never return `None`, however, it may make your code easier to test if an + /// implementation is provided. fn parse_loaded(&self, response: &http::Response) -> Self::Output; } @@ -77,3 +83,39 @@ where self.parse(response) } } + +#[cfg(test)] +mod test { + use crate::response::ParseHttpResponse; + use bytes::Bytes; + use http::Response; + use http_body::Body; + use std::mem; + + #[test] + fn supports_streaming_body() { + struct S3GetObject { + pub body: B, + } + + struct S3GetObjectParser; + + impl ParseHttpResponse for S3GetObjectParser + where + B: Default + Body, + { + type Output = S3GetObject; + + fn parse_unloaded(&self, response: &mut Response) -> Option { + // For responses that pass on the body, use mem::take to leave behind an empty + // body + let body = mem::take(response.body_mut()); + Some(S3GetObject { body }) + } + + fn parse_loaded(&self, _response: &Response) -> Self::Output { + unimplemented!() + } + } + } +} diff --git a/rust-runtime/smithy-http/src/result.rs b/rust-runtime/smithy-http/src/result.rs new file mode 100644 index 0000000000..bfa8e76b31 --- /dev/null +++ b/rust-runtime/smithy-http/src/result.rs @@ -0,0 +1,59 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +use std::error::Error; +use std::fmt::Debug; + +type BoxError = Box; + +/// Body type when a response is returned. Currently, the only use case is introspecting errors +/// so it is simply `Debug`. This is an area of potential design iteration. +// pub type Body = Pin> + Send + Sync>>; + +/// Successful Sdk Result +/// +/// Typically, transport implementations will type alias (or entirely wrap / transform) this type +/// plugging in a concrete body implementation, eg: +/// ```rust +/// # mod hyper { +/// # pub struct Body; +/// # } +/// type SdkSuccess = smithy_http::result::SdkSuccess; +/// ``` +#[derive(Debug)] +pub struct SdkSuccess { + pub raw: http::Response, + pub parsed: O, +} + +/// Failing Sdk Result +/// +/// Typically, transport implementations will type alias (or entirely wrap / transform) this type +/// by specifying a concrete body implementation: +/// ```rust +/// # mod hyper { +/// # pub struct Body; +/// # } +/// type SdkError = smithy_http::result::SdkError; +/// ``` +#[derive(Debug)] +pub enum SdkError { + /// The request failed during construction. It was not dispatched over the network. + ConstructionFailure(BoxError), + + /// The request failed during dispatch. An HTTP response was not received. The request MAY + /// have been sent. + DispatchFailure(BoxError), + + /// A response was received but it was not parseable according the the protocol (for example + /// the server hung up while the body was being read) + ResponseError { + raw: http::Response, + err: BoxError, + }, + + /// An error response was received from the service + ServiceError { raw: http::Response, err: E }, +}