Unverified Commit 68984dc4 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Refactor middleware to use new `operation::Response` instead of `http::Response<SdkBody>` (#635) 

* Refactor Smithy service tower to include a property bag with the response

* Add doc comments and rename functions on operation Request/Response

* Fix codegen

* Update CHANGELOG

* Attach PR number to CHANGELOG

* Fix test compile error

* CR feedback

* Fix AWS runtime tests

* Fix doc comment link

* Fix API gateway customization

* Fix AWS integration tests and update design doc

* Make it possible to run IAM test outside CI and fix it
parent 8aa3a74a

CHANGELOG.md

+3 −0
Original line number Diff line number Diff line
@@ -3,6 +3,9 @@ vNext (Month Day Year) 


**Breaking changes**



- (#635) The `config()`, `config_mut()`, `request()`, and `request_mut()` methods on `operation::Request` have been renamed to `properties()`, `properties_mut()`, `http()`, and `http_mut()` respectively.

- (#635) The `Response` type on Tower middleware has been changed from `http::Response<SdkBody>` to `operation::Response`. The HTTP response is still available from the `operation::Response` using its `http()` and `http_mut()` methods.

- (#635) The `ParseHttpResponse` trait's `parse_unloaded()` method now takes an `operation::Response` rather than an `http::Response<SdkBody>`.

- (#626) `ParseHttpResponse` no longer has a generic argument for the body type, but instead, always uses `SdkBody`. This may cause compilation failures for you if you are using Smithy generated types to parse JSON or XML without using a client to request data from a service. The fix should be as simple as removing `<SdkBody>` in the example below:



  Before:

aws/rust-runtime/aws-auth/src/middleware.rs

+5 −5
Original line number Diff line number Diff line
@@ -73,8 +73,8 @@ impl AsyncMapRequest for CredentialsStage { 
    fn apply(&self, mut request: Request) -> BoxFuture<Result<Request, Self::Error>> {

        Box::pin(async move {

            let provider = {

                let config = request.config();

                let credential_provider = config

                let properties = request.properties();

                let credential_provider = properties

                    .get::<CredentialsProvider>()

                    .ok_or(CredentialsStageError::MissingCredentialsProvider)?;

                // we need to enable releasing the config lock so that we don't hold the config
@@ -83,7 +83,7 @@ impl AsyncMapRequest for CredentialsStage { 
            };

            let cred_future = { provider.provide_credentials() };

            let credentials = cred_future.await?;

            request.config_mut().insert(credentials);

            request.properties_mut().insert(credentials);

            Ok(request)

        })

    }
@@ -112,7 +112,7 @@ mod tests { 
    async fn async_map_request_apply_populates_credentials() {

        let mut req = operation::Request::new(http::Request::new(SdkBody::from("some body")));

        set_provider(

            &mut req.config_mut(),

            &mut req.properties_mut(),

            Arc::new(Credentials::from_keys("test", "test", None)),

        );

        let req = CredentialsStage::new()
@@ -120,7 +120,7 @@ mod tests { 
            .await

            .expect("credential provider is in the bag; should succeed");

        assert!(

            req.config().get::<Credentials>().is_some(),

            req.properties().get::<Credentials>().is_some(),

            "it should set credentials on the request config"

        );

    }

aws/rust-runtime/aws-endpoint/src/lib.rs

+22 −22
Original line number Diff line number Diff line
@@ -145,12 +145,12 @@ impl ResolveAwsEndpoint for Endpoint { 
}



type AwsEndpointResolver = Arc<dyn ResolveAwsEndpoint>;

pub fn get_endpoint_resolver(config: &PropertyBag) -> Option<&AwsEndpointResolver> {

    config.get()

pub fn get_endpoint_resolver(properties: &PropertyBag) -> Option<&AwsEndpointResolver> {

    properties.get()

}



pub fn set_endpoint_resolver(config: &mut PropertyBag, provider: AwsEndpointResolver) {

    config.insert(provider);

pub fn set_endpoint_resolver(properties: &mut PropertyBag, provider: AwsEndpointResolver) {

    properties.insert(provider);

}



/// Middleware Stage to Add an Endpoint to a Request
@@ -182,10 +182,10 @@ impl MapRequest for AwsEndpointStage { 
    type Error = AwsEndpointStageError;



    fn apply(&self, request: Request) -> Result<Request, Self::Error> {

        request.augment(|mut http_req, config| {

        request.augment(|mut http_req, props| {

            let provider =

                get_endpoint_resolver(config).ok_or(AwsEndpointStageError::NoEndpointResolver)?;

            let region = config

                get_endpoint_resolver(props).ok_or(AwsEndpointStageError::NoEndpointResolver)?;

            let region = props

                .get::<Region>()

                .ok_or(AwsEndpointStageError::NoRegion)?;

            let endpoint = provider
@@ -195,13 +195,13 @@ impl MapRequest for AwsEndpointStage { 
                .credential_scope

                .region

                .unwrap_or_else(|| region.clone().into());

            config.insert::<SigningRegion>(signing_region);

            props.insert::<SigningRegion>(signing_region);

            if let Some(signing_service) = endpoint.credential_scope.service {

                config.insert::<SigningService>(signing_service);

                props.insert::<SigningService>(signing_service);

            }

            endpoint

                .endpoint

                .set_endpoint(http_req.uri_mut(), config.get::<EndpointPrefix>());

                .set_endpoint(http_req.uri_mut(), props.get::<EndpointPrefix>());

            // host is only None if authority is not. `set_endpoint` guarantees that authority is not None

            let host = http_req

                .uri()
@@ -243,18 +243,18 @@ mod test { 
        let region = Region::new("us-east-1");

        let mut req = operation::Request::new(req);

        {

            let mut conf = req.config_mut();

            conf.insert(region.clone());

            conf.insert(SigningService::from_static("kinesis"));

            set_endpoint_resolver(&mut conf, provider);

            let mut props = req.properties_mut();

            props.insert(region.clone());

            props.insert(SigningService::from_static("kinesis"));

            set_endpoint_resolver(&mut props, provider);

        };

        let req = AwsEndpointStage.apply(req).expect("should succeed");

        assert_eq!(

            req.config().get(),

            req.properties().get(),

            Some(&SigningRegion::from(region.clone()))

        );

        assert_eq!(

            req.config().get(),

            req.properties().get(),

            Some(&SigningService::from_static("kinesis"))

        );
@@ -284,18 +284,18 @@ mod test { 
        let region = Region::new("us-east-1");

        let mut req = operation::Request::new(req);

        {

            let mut conf = req.config_mut();

            conf.insert(region.clone());

            conf.insert(SigningService::from_static("kinesis"));

            set_endpoint_resolver(&mut conf, provider);

            let mut props = req.properties_mut();

            props.insert(region.clone());

            props.insert(SigningService::from_static("kinesis"));

            set_endpoint_resolver(&mut props, provider);

        };

        let req = AwsEndpointStage.apply(req).expect("should succeed");

        assert_eq!(

            req.config().get(),

            req.properties().get(),

            Some(&SigningRegion::from(Region::new("us-east-override")))

        );

        assert_eq!(

            req.config().get(),

            req.properties().get(),

            Some(&SigningService::from_static("qldb-override"))

        );

    }

aws/rust-runtime/aws-http/src/lib.rs

+4 −2
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ where 
            Err(_) => return RetryKind::NotRetryable,

        };

        if let Some(retry_after_delay) = response

            .http()

            .headers()

            .get("x-amz-retry-after")

            .and_then(|header| header.to_str().ok())
@@ -82,7 +83,7 @@ where 
                return RetryKind::Error(ErrorKind::TransientError);

            }

        };

        if TRANSIENT_ERROR_STATUS_CODES.contains(&response.status().as_u16()) {

        if TRANSIENT_ERROR_STATUS_CODES.contains(&response.http().status().as_u16()) {

            return RetryKind::Error(ErrorKind::TransientError);

        };

        // TODO: is IDPCommunicationError modeled yet?
@@ -94,6 +95,7 @@ where 
mod test {

    use crate::AwsErrorRetryPolicy;

    use smithy_http::body::SdkBody;

    use smithy_http::operation;

    use smithy_http::result::{SdkError, SdkSuccess};

    use smithy_http::retry::ClassifyResponse;

    use smithy_types::retry::{ErrorKind, ProvideErrorKind, RetryKind};
@@ -131,7 +133,7 @@ mod test { 
    ) -> Result<SdkSuccess<()>, SdkError<E>> {

        Err(SdkError::ServiceError {

            err,

            raw: raw.map(|b| SdkBody::from(b)),

            raw: operation::Response::new(raw.map(|b| SdkBody::from(b))),

        })

    }

aws/rust-runtime/aws-http/src/user_agent.rs

+2 −2
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ use thiserror::Error; 


/// AWS User Agent

///

/// Ths struct should be inserted into the [`PropertyBag`](smithy_http::operation::Request::config)

/// Ths struct should be inserted into the [`PropertyBag`](smithy_http::operation::Request::properties)

/// during operation construction. [`UserAgentStage`](UserAgentStage) reads `AwsUserAgent`

/// from the property bag and sets the `User-Agent` and `x-amz-user-agent` headers.

pub struct AwsUserAgent {
@@ -298,7 +298,7 @@ mod test { 
            .apply(req)

            .expect_err("adding UA should fail without a UA set");

        let mut req = operation::Request::new(http::Request::new(SdkBody::from("some body")));

        req.config_mut()

        req.properties_mut()

            .insert(AwsUserAgent::new_from_environment(ApiMetadata {

                service_id: "dynamodb".into(),

                version: "0.123",