From 68984dc4086454d657978fde4339987061d2952a Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 5 Aug 2021 17:44:58 -0700 Subject: [PATCH] Refactor middleware to use new `operation::Response` instead of `http::Response` (#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 --- CHANGELOG.md | 3 + aws/rust-runtime/aws-auth/src/middleware.rs | 10 +- aws/rust-runtime/aws-endpoint/src/lib.rs | 44 ++++---- aws/rust-runtime/aws-http/src/lib.rs | 6 +- aws/rust-runtime/aws-http/src/user_agent.rs | 4 +- aws/rust-runtime/aws-hyper/tests/e2e_test.rs | 8 +- .../aws-sig-auth/src/middleware.rs | 4 +- .../smithy/rustsdk/AwsEndpointDecorator.kt | 2 +- .../smithy/rustsdk/CredentialProviders.kt | 2 +- .../amazon/smithy/rustsdk/RegionDecorator.kt | 2 +- .../smithy/rustsdk/SigV4SigningDecorator.kt | 6 +- .../smithy/rustsdk/UserAgentDecorator.kt | 2 +- .../apigateway/ApiGatewayDecorator.kt | 2 +- aws/sdk/integration-tests/iam/Cargo.toml | 22 ++++ aws/sdk/integration-tests/iam/src/lib.rs | 4 + .../iam/tests/resolve-global-endpoint.rs | 4 +- aws/sdk/integration-tests/kms/Cargo.toml | 3 +- .../kms/tests/integration.rs | 8 +- .../kms/tests/sensitive-it.rs | 6 +- .../integration-tests/qldbsession/Cargo.toml | 3 +- .../qldbsession/tests/integration.rs | 4 +- aws/sdk/integration-tests/s3/Cargo.toml | 1 + .../integration-tests/s3/tests/signing-it.rs | 4 +- .../customizations/EndpointPrefixGenerator.kt | 2 +- .../generators/HttpProtocolTestGenerator.kt | 9 +- .../protocols/HttpBoundProtocolGenerator.kt | 18 +-- .../generators/EndpointTraitBindingsTest.kt | 4 +- design/src/transport/operation.md | 10 +- rust-runtime/smithy-client/src/bounds.rs | 4 +- rust-runtime/smithy-client/src/erase.rs | 4 +- .../smithy-client/src/static_tests.rs | 11 +- .../smithy-http-tower/src/dispatch.rs | 7 +- .../smithy-http-tower/src/parse_response.rs | 8 +- rust-runtime/smithy-http/src/middleware.rs | 27 +++-- rust-runtime/smithy-http/src/operation.rs | 105 ++++++++++++++---- rust-runtime/smithy-http/src/response.rs | 19 ++-- rust-runtime/smithy-http/src/result.rs | 16 ++- 37 files changed, 255 insertions(+), 143 deletions(-) create mode 100644 aws/sdk/integration-tests/iam/Cargo.toml create mode 100644 aws/sdk/integration-tests/iam/src/lib.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 89bf4331dc..1308646687 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` 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`. - (#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 `` in the example below: Before: diff --git a/aws/rust-runtime/aws-auth/src/middleware.rs b/aws/rust-runtime/aws-auth/src/middleware.rs index 6ca55c7f91..aa50e7631c 100644 --- a/aws/rust-runtime/aws-auth/src/middleware.rs +++ b/aws/rust-runtime/aws-auth/src/middleware.rs @@ -73,8 +73,8 @@ impl AsyncMapRequest for CredentialsStage { fn apply(&self, mut request: Request) -> BoxFuture> { Box::pin(async move { let provider = { - let config = request.config(); - let credential_provider = config + let properties = request.properties(); + let credential_provider = properties .get::() .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::().is_some(), + req.properties().get::().is_some(), "it should set credentials on the request config" ); } diff --git a/aws/rust-runtime/aws-endpoint/src/lib.rs b/aws/rust-runtime/aws-endpoint/src/lib.rs index 769501571e..5cfaf9fddd 100644 --- a/aws/rust-runtime/aws-endpoint/src/lib.rs +++ b/aws/rust-runtime/aws-endpoint/src/lib.rs @@ -145,12 +145,12 @@ impl ResolveAwsEndpoint for Endpoint { } type AwsEndpointResolver = Arc; -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.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::() .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::(signing_region); + props.insert::(signing_region); if let Some(signing_service) = endpoint.credential_scope.service { - config.insert::(signing_service); + props.insert::(signing_service); } endpoint .endpoint - .set_endpoint(http_req.uri_mut(), config.get::()); + .set_endpoint(http_req.uri_mut(), props.get::()); // 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")) ); } diff --git a/aws/rust-runtime/aws-http/src/lib.rs b/aws/rust-runtime/aws-http/src/lib.rs index 3bd0f30663..6546b7dd67 100644 --- a/aws/rust-runtime/aws-http/src/lib.rs +++ b/aws/rust-runtime/aws-http/src/lib.rs @@ -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, SdkError> { Err(SdkError::ServiceError { err, - raw: raw.map(|b| SdkBody::from(b)), + raw: operation::Response::new(raw.map(|b| SdkBody::from(b))), }) } diff --git a/aws/rust-runtime/aws-http/src/user_agent.rs b/aws/rust-runtime/aws-http/src/user_agent.rs index ac811a95bb..dde60b5b88 100644 --- a/aws/rust-runtime/aws-http/src/user_agent.rs +++ b/aws/rust-runtime/aws-http/src/user_agent.rs @@ -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", diff --git a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs index 00dd0f97a6..fd6ee841a9 100644 --- a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs +++ b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs @@ -14,7 +14,7 @@ use aws_types::region::Region; use aws_types::SigningService; use bytes::Bytes; use http::header::{AUTHORIZATION, HOST, USER_AGENT}; -use http::{Response, Uri}; +use http::{self, Uri}; use smithy_client::test_connection::TestConnection; use smithy_http::body::SdkBody; use smithy_http::operation; @@ -57,15 +57,15 @@ impl ProvideErrorKind for OperationError { impl ParseHttpResponse for TestOperationParser { type Output = Result; - fn parse_unloaded(&self, response: &mut Response) -> Option { - if response.status().is_success() { + fn parse_unloaded(&self, response: &mut operation::Response) -> Option { + if response.http().status().is_success() { Some(Ok("Hello!".to_string())) } else { Some(Err(OperationError)) } } - fn parse_loaded(&self, _response: &Response) -> Self::Output { + fn parse_loaded(&self, _response: &http::Response) -> Self::Output { Ok("Hello!".to_string()) } } diff --git a/aws/rust-runtime/aws-sig-auth/src/middleware.rs b/aws/rust-runtime/aws-sig-auth/src/middleware.rs index 8c200af6be..5c83fd6b96 100644 --- a/aws/rust-runtime/aws-sig-auth/src/middleware.rs +++ b/aws/rust-runtime/aws-sig-auth/src/middleware.rs @@ -151,13 +151,13 @@ mod test { ); let mut config = OperationSigningConfig::default_config(); config.signing_options.content_sha256_header = true; - req.config_mut().insert(config); + req.properties_mut().insert(config); errs.push( signer .apply(req.try_clone().expect("can clone")) .expect_err("no cred provider"), ); - req.config_mut() + req.properties_mut() .insert(Credentials::from_keys("AKIAfoo", "bar", None)); let req = signer.apply(req).expect("signing succeeded"); // make sure we got the correct error types in any order diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt index 7812acd4bd..d6e68d2ba6 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsEndpointDecorator.kt @@ -115,7 +115,7 @@ class EndpointResolverFeature(private val runtimeConfig: RuntimeConfig, private is OperationSection.MutateRequest -> writable { rust( """ - #T::set_endpoint_resolver(&mut ${section.request}.config_mut(), ${section.config}.endpoint_resolver.clone()); + #T::set_endpoint_resolver(&mut ${section.request}.properties_mut(), ${section.config}.endpoint_resolver.clone()); """, runtimeConfig.awsEndpointDependency().asType() ) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt index f33d95fd40..30d9536bf2 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt @@ -90,7 +90,7 @@ class CredentialsProviderFeature(private val runtimeConfig: RuntimeConfig) : Ope is OperationSection.MutateRequest -> writable { rust( """ - #T(&mut ${section.request}.config_mut(), ${section.config}.credentials_provider.clone()); + #T(&mut ${section.request}.properties_mut(), ${section.config}.credentials_provider.clone()); """, setProvider(runtimeConfig) ) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt index 5a262981d6..28f84520b0 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt @@ -110,7 +110,7 @@ class RegionConfigPlugin : OperationCustomization() { rust( """ if let Some(region) = &${section.config}.region { - ${section.request}.config_mut().insert(region.clone()); + ${section.request}.properties_mut().insert(region.clone()); } """ ) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt index b754d2e9f4..f0b23836a1 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt @@ -119,14 +119,14 @@ class SigV4SigningFeature( if (operation.hasTrait()) { rust("signing_config.signing_options.content_sha256_header = true;") rustTemplate( - "${section.request}.config_mut().insert(#{sig_auth}::signer::SignableBody::UnsignedPayload);", + "${section.request}.properties_mut().insert(#{sig_auth}::signer::SignableBody::UnsignedPayload);", *codegenScope ) } rustTemplate( """ - ${section.request}.config_mut().insert(signing_config); - ${section.request}.config_mut().insert(#{aws_types}::SigningService::from_static(${section.config}.signing_service())); + ${section.request}.properties_mut().insert(signing_config); + ${section.request}.properties_mut().insert(#{aws_types}::SigningService::from_static(${section.config}.signing_service())); """, *codegenScope ) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt index ea15c89fb2..c8379c8e01 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt @@ -67,7 +67,7 @@ class UserAgentFeature(private val runtimeConfig: RuntimeConfig) : OperationCust is OperationSection.MutateRequest -> writable { rust( """ - ${section.request}.config_mut().insert(#T::AwsUserAgent::new_from_environment(crate::API_METADATA.clone())); + ${section.request}.properties_mut().insert(#T::AwsUserAgent::new_from_environment(crate::API_METADATA.clone())); """, runtimeConfig.userAgentModule() ) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt index 40d4eb671a..8ca736cd7d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/apigateway/ApiGatewayDecorator.kt @@ -40,7 +40,7 @@ class ApiGatewayAddAcceptHeader : OperationCustomization() { is OperationSection.MutateRequest -> writable { rust( """${section.request} - .request_mut() + .http_mut() .headers_mut() .insert("Accept", #T::HeaderValue::from_static("application/json"));""", RuntimeType.http diff --git a/aws/sdk/integration-tests/iam/Cargo.toml b/aws/sdk/integration-tests/iam/Cargo.toml new file mode 100644 index 0000000000..e137e55ac6 --- /dev/null +++ b/aws/sdk/integration-tests/iam/Cargo.toml @@ -0,0 +1,22 @@ +# This Cargo.toml is unused in generated code. It exists solely to enable these tests to compile in-situ +[package] +name = "iam-tests" +version = "0.1.0" +authors = ["AWS Rust SDK Team "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +aws-sdk-iam = { path = "../../build/aws-sdk/iam" } +aws-endpoint = { path = "../../build/aws-sdk/aws-endpoint" } +smithy-client = { path = "../../build/aws-sdk/smithy-client", features = ["test-util"] } +smithy-http = { path = "../../build/aws-sdk/smithy-http" } +tracing-subscriber = "0.2.18" + +[dev-dependencies] +tokio = { version = "1", features = ["full"]} +http = "0.2.3" +bytes = "1" +aws-hyper = { path = "../../build/aws-sdk/aws-hyper"} +aws-http = { path = "../../build/aws-sdk/aws-http"} diff --git a/aws/sdk/integration-tests/iam/src/lib.rs b/aws/sdk/integration-tests/iam/src/lib.rs new file mode 100644 index 0000000000..c6d888f029 --- /dev/null +++ b/aws/sdk/integration-tests/iam/src/lib.rs @@ -0,0 +1,4 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ diff --git a/aws/sdk/integration-tests/iam/tests/resolve-global-endpoint.rs b/aws/sdk/integration-tests/iam/tests/resolve-global-endpoint.rs index 19a0995f12..9ea845204c 100644 --- a/aws/sdk/integration-tests/iam/tests/resolve-global-endpoint.rs +++ b/aws/sdk/integration-tests/iam/tests/resolve-global-endpoint.rs @@ -15,8 +15,8 @@ fn correct_endpoint_resolver() { .unwrap() .make_operation(&conf) .expect("valid operation"); - let conf = operation.config(); - let resolver = get_endpoint_resolver(&conf).expect("operation should have endpoint resolver"); + let props = operation.properties(); + let resolver = get_endpoint_resolver(&props).expect("operation should have endpoint resolver"); // test regular endpoint { let ep = resolver diff --git a/aws/sdk/integration-tests/kms/Cargo.toml b/aws/sdk/integration-tests/kms/Cargo.toml index e2bd29e6fd..a6d7d91d9e 100644 --- a/aws/sdk/integration-tests/kms/Cargo.toml +++ b/aws/sdk/integration-tests/kms/Cargo.toml @@ -9,10 +9,11 @@ edition = "2018" [dependencies] aws-sdk-kms = { path = "../../build/aws-sdk/kms" } +smithy-client = { path = "../../build/aws-sdk/smithy-client", features = ["test-util"] } smithy-http = { path = "../../build/aws-sdk/smithy-http" } smithy-types = { path = "../../build/aws-sdk/smithy-types" } http = "0.2.3" -aws-hyper = { path = "../../build/aws-sdk/aws-hyper", features = ["test-util"] } +aws-hyper = { path = "../../build/aws-sdk/aws-hyper" } aws-auth = { path = "../../build/aws-sdk/aws-auth" } aws-http = { path = "../../build/aws-sdk/aws-http" } tokio = { version = "1", features = ["full"]} diff --git a/aws/sdk/integration-tests/kms/tests/integration.rs b/aws/sdk/integration-tests/kms/tests/integration.rs index d0472c6edf..ecb2dd80ac 100644 --- a/aws/sdk/integration-tests/kms/tests/integration.rs +++ b/aws/sdk/integration-tests/kms/tests/integration.rs @@ -89,9 +89,9 @@ async fn generate_random() { .unwrap() .make_operation(&conf) .expect("valid operation"); - op.config_mut() + op.properties_mut() .insert(UNIX_EPOCH + Duration::from_secs(1614952162)); - op.config_mut().insert(AwsUserAgent::for_tests()); + op.properties_mut().insert(AwsUserAgent::for_tests()); let resp = client.call(op).await.expect("request should succeed"); // primitive checksum assert_eq!( @@ -181,9 +181,9 @@ async fn generate_random_keystore_not_found() { .make_operation(&conf) .expect("valid operation"); - op.config_mut() + op.properties_mut() .insert(UNIX_EPOCH + Duration::from_secs(1614955644)); - op.config_mut().insert(AwsUserAgent::for_tests()); + op.properties_mut().insert(AwsUserAgent::for_tests()); let client = Client::new(conn.clone()); let err = client.call(op).await.expect_err("key store doesn't exist"); let inner = match err { diff --git a/aws/sdk/integration-tests/kms/tests/sensitive-it.rs b/aws/sdk/integration-tests/kms/tests/sensitive-it.rs index dc1b754e50..c6ce187912 100644 --- a/aws/sdk/integration-tests/kms/tests/sensitive-it.rs +++ b/aws/sdk/integration-tests/kms/tests/sensitive-it.rs @@ -11,7 +11,7 @@ use kms::operation::{CreateAlias, GenerateRandom}; use kms::output::GenerateRandomOutput; use kms::Blob; use smithy_http::body::SdkBody; -use smithy_http::operation::Parts; +use smithy_http::operation::{self, Parts}; use smithy_http::response::ParseStrictResponse; use smithy_http::result::SdkError; use smithy_http::retry::ClassifyResponse; @@ -88,7 +88,7 @@ fn errors_are_retryable() { .parse(&http_response) .map_err(|e| SdkError::ServiceError { err: e, - raw: http_response.map(SdkBody::from), + raw: operation::Response::new(http_response.map(SdkBody::from)), }); let retry_kind = op.retry_policy.classify(err.as_ref()); assert_eq!(retry_kind, RetryKind::Error(ErrorKind::ThrottlingError)); @@ -106,7 +106,7 @@ fn unmodeled_errors_are_retryable() { .parse(&http_response) .map_err(|e| SdkError::ServiceError { err: e, - raw: http_response.map(SdkBody::from), + raw: operation::Response::new(http_response.map(SdkBody::from)), }); let retry_kind = op.retry_policy.classify(err.as_ref()); assert_eq!(retry_kind, RetryKind::Error(ErrorKind::ThrottlingError)); diff --git a/aws/sdk/integration-tests/qldbsession/Cargo.toml b/aws/sdk/integration-tests/qldbsession/Cargo.toml index 84917734bb..35b3411007 100644 --- a/aws/sdk/integration-tests/qldbsession/Cargo.toml +++ b/aws/sdk/integration-tests/qldbsession/Cargo.toml @@ -9,10 +9,11 @@ edition = "2018" [dependencies] aws-sdk-qldbsession = { path = "../../build/aws-sdk/qldbsession" } +smithy-client = { path = "../../build/aws-sdk/smithy-client", features = ["test-util"] } smithy-http = { path = "../../build/aws-sdk/smithy-http" } smithy-types = { path = "../../build/aws-sdk/smithy-types" } http = "0.2.3" -aws-hyper = { path = "../../build/aws-sdk/aws-hyper", features = ["test-util"] } +aws-hyper = { path = "../../build/aws-sdk/aws-hyper" } aws-auth = { path = "../../build/aws-sdk/aws-auth" } aws-http = { path = "../../build/aws-sdk/aws-http" } tokio = { version = "1", features = ["full"]} diff --git a/aws/sdk/integration-tests/qldbsession/tests/integration.rs b/aws/sdk/integration-tests/qldbsession/tests/integration.rs index fa1ffc7f47..5bda783989 100644 --- a/aws/sdk/integration-tests/qldbsession/tests/integration.rs +++ b/aws/sdk/integration-tests/qldbsession/tests/integration.rs @@ -61,9 +61,9 @@ async fn signv4_use_correct_service_name() { .make_operation(&conf) .expect("valid operation"); // Fix the request time and user agent so the headers are stable - op.config_mut() + op.properties_mut() .insert(UNIX_EPOCH + Duration::from_secs(1614952162)); - op.config_mut().insert(AwsUserAgent::for_tests()); + op.properties_mut().insert(AwsUserAgent::for_tests()); let _ = client.call(op).await.expect("request should succeed"); diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 356fc5c6e9..e78b9b96ba 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" [dependencies] aws-sdk-s3 = { path = "../../build/aws-sdk/s3" } +smithy-client = { path = "../../build/aws-sdk/smithy-client", features = ["test-util"] } smithy-http = { path = "../../build/aws-sdk/smithy-http" } tracing-subscriber = "0.2.18" diff --git a/aws/sdk/integration-tests/s3/tests/signing-it.rs b/aws/sdk/integration-tests/s3/tests/signing-it.rs index fc6090bbd2..81d9ac110b 100644 --- a/aws/sdk/integration-tests/s3/tests/signing-it.rs +++ b/aws/sdk/integration-tests/s3/tests/signing-it.rs @@ -37,9 +37,9 @@ async fn test_signer() -> Result<(), aws_sdk_s3::Error> { .unwrap() .make_operation(&conf) .unwrap(); - op.config_mut() + op.properties_mut() .insert(UNIX_EPOCH + Duration::from_secs(1624036048)); - op.config_mut().insert(AwsUserAgent::for_tests()); + op.properties_mut().insert(AwsUserAgent::for_tests()); client.call(op).await.expect_err("empty response"); for req in conn.requests().iter() { diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/EndpointPrefixGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/EndpointPrefixGenerator.kt index d8445476ef..9991ddffee 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/EndpointPrefixGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/EndpointPrefixGenerator.kt @@ -35,7 +35,7 @@ class EndpointPrefixGenerator(private val protocolConfig: ProtocolConfig, privat endpointTraitBindings.render(this, "self") } rustBlock("match endpoint_prefix") { - rust("Ok(prefix) => { request.config_mut().insert(prefix); },") + rust("Ok(prefix) => { request.properties_mut().insert(prefix); },") rust("Err(err) => return Err(${buildError.serializationError(this, "err")})") } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt index 95d7178fe4..06e2b27e93 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt @@ -258,7 +258,7 @@ class HttpProtocolTestGenerator( writeInline("let expected_output =") instantiator.render(this, expectedShape, testCase.params) write(";") - write("let mut http_response = #T::new()", RuntimeType.HttpResponseBuilder) + write("let http_response = #T::new()", RuntimeType.HttpResponseBuilder) testCase.headers.forEach { (key, value) -> writeWithNoFormatting(".header(${key.dq()}, ${value.dq()})") } @@ -270,12 +270,17 @@ class HttpProtocolTestGenerator( """, RuntimeType.sdkBody(runtimeConfig = protocolConfig.runtimeConfig) ) + write( + "let mut op_response = #T::new(http_response);", + RuntimeType.operationModule(protocolConfig.runtimeConfig).member("Response") + ) rustTemplate( """ use #{parse_http_response}; let parser = #{op}::new(); - let parsed = parser.parse_unloaded(&mut http_response); + let parsed = parser.parse_unloaded(&mut op_response); let parsed = parsed.unwrap_or_else(|| { + let (http_response, _) = op_response.into_parts(); let http_response = http_response.map(|body|#{bytes}::copy_from_slice(body.bytes().unwrap())); <#{op} as #{parse_http_response}>::parse_loaded(&parser, &http_response) }); diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt index 50faad534e..b3b5ac3d8e 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt @@ -80,7 +80,8 @@ class HttpBoundProtocolGenerator( private val codegenScope = arrayOf( "ParseStrict" to RuntimeType.parseStrict(runtimeConfig), "ParseResponse" to RuntimeType.parseResponse(runtimeConfig), - "Response" to RuntimeType.Http("Response"), + "http" to RuntimeType.http, + "operation" to RuntimeType.operationModule(runtimeConfig), "Bytes" to RuntimeType.Bytes, "SdkBody" to RuntimeType.sdkBody(runtimeConfig), "BuildError" to runtimeConfig.operationBuildError() @@ -221,7 +222,7 @@ class HttpBoundProtocolGenerator( """ impl #{ParseStrict} for $operationName { type Output = std::result::Result<#{O}, #{E}>; - fn parse(&self, response: &#{Response}<#{Bytes}>) -> Self::Output { + fn parse(&self, response: &#{http}::Response<#{Bytes}>) -> Self::Output { if !response.status().is_success() && response.status().as_u16() != $successCode { #{parse_error}(response) } else { @@ -247,14 +248,14 @@ class HttpBoundProtocolGenerator( """ impl #{ParseResponse} for $operationName { type Output = std::result::Result<#{O}, #{E}>; - fn parse_unloaded(&self, response: &mut http::Response<#{SdkBody}>) -> Option { + fn parse_unloaded(&self, response: &mut #{operation}::Response) -> Option { // This is an error, defer to the non-streaming parser - if !response.status().is_success() && response.status().as_u16() != $successCode { + if !response.http().status().is_success() && response.http().status().as_u16() != $successCode { return None; } Some(#{parse_streaming_response}(response)) } - fn parse_loaded(&self, response: &http::Response<#{Bytes}>) -> Self::Output { + fn parse_loaded(&self, response: &#{http}::Response<#{Bytes}>) -> Self::Output { // if streaming, we only hit this case if its an error #{parse_error}(response) } @@ -276,7 +277,7 @@ class HttpBoundProtocolGenerator( return RuntimeType.forInlineFun(fnName, "operation_deser") { Attribute.Custom("allow(clippy::unnecessary_wraps)").render(it) it.rustBlockTemplate( - "pub fn $fnName(response: &#{Response}<#{Bytes}>) -> std::result::Result<#{O}, #{E}>", + "pub fn $fnName(response: &#{http}::Response<#{Bytes}>) -> std::result::Result<#{O}, #{E}>", *codegenScope, "O" to outputSymbol, "E" to errorSymbol @@ -350,11 +351,12 @@ class HttpBoundProtocolGenerator( return RuntimeType.forInlineFun(fnName, "operation_deser") { Attribute.Custom("allow(clippy::unnecessary_wraps)").render(it) it.rustBlockTemplate( - "pub fn $fnName(response: &mut #{Response}<#{SdkBody}>) -> std::result::Result<#{O}, #{E}>", + "pub fn $fnName(op_response: &mut #{operation}::Response) -> std::result::Result<#{O}, #{E}>", *codegenScope, "O" to outputSymbol, "E" to errorSymbol ) { + write("let response = op_response.http_mut();") withBlock("Ok({", "})") { renderShapeParser( operationShape, @@ -375,7 +377,7 @@ class HttpBoundProtocolGenerator( return RuntimeType.forInlineFun(fnName, "operation_deser") { Attribute.Custom("allow(clippy::unnecessary_wraps)").render(it) it.rustBlockTemplate( - "pub fn $fnName(response: &#{Response}<#{Bytes}>) -> std::result::Result<#{O}, #{E}>", + "pub fn $fnName(response: &#{http}::Response<#{Bytes}>) -> std::result::Result<#{O}, #{E}>", *codegenScope, "O" to outputSymbol, "E" to errorSymbol diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EndpointTraitBindingsTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EndpointTraitBindingsTest.kt index 99a456ce4a..b65a1e321c 100644 --- a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EndpointTraitBindingsTest.kt +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/EndpointTraitBindingsTest.kt @@ -151,8 +151,8 @@ internal class EndpointTraitBindingsTest { .greeting("hello") .build().expect("valid operation") .make_operation(&conf).expect("hello is a valid prefix"); - let op_conf = op.config(); - let prefix = op_conf.get::() + let properties = op.properties(); + let prefix = properties.get::() .expect("prefix should be in config") .as_str(); assert_eq!(prefix, "test123.hello."); diff --git a/design/src/transport/operation.md b/design/src/transport/operation.md index 92bc37ee1c..9ca8a4dfc9 100644 --- a/design/src/transport/operation.md +++ b/design/src/transport/operation.md @@ -24,8 +24,8 @@ pub struct Operation { } pub struct Request { - base: http::Request, - configuration: PropertyBag, + inner: http::Request, + properties: PropertyBag, } ``` @@ -46,9 +46,9 @@ pub fn build(self, config: &dynamodb::config::Config) -> Operation, + Response = smithy_http::operation::Response, Error = smithy_http_tower::SendOperationError, Future = ::Future, > @@ -77,7 +77,7 @@ impl SmithyMiddlewareService for T where T: Service< smithy_http::operation::Request, - Response = http::Response, + Response = smithy_http::operation::Response, Error = smithy_http_tower::SendOperationError, >, T::Future: Send + 'static, diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs index e3176cc564..726584765a 100644 --- a/rust-runtime/smithy-client/src/erase.rs +++ b/rust-runtime/smithy-client/src/erase.rs @@ -178,7 +178,7 @@ pub struct DynMiddleware( BoxCloneLayer< smithy_http_tower::dispatch::DispatchService, smithy_http::operation::Request, - http::Response, + smithy_http::operation::Response, smithy_http_tower::SendOperationError, >, ); @@ -199,7 +199,7 @@ impl DynMiddleware { impl Layer> for DynMiddleware { type Service = BoxCloneService< smithy_http::operation::Request, - http::Response, + smithy_http::operation::Response, smithy_http_tower::SendOperationError, >; diff --git a/rust-runtime/smithy-client/src/static_tests.rs b/rust-runtime/smithy-client/src/static_tests.rs index e6cc6c95d2..4b4fc7c8b7 100644 --- a/rust-runtime/smithy-client/src/static_tests.rs +++ b/rust-runtime/smithy-client/src/static_tests.rs @@ -1,7 +1,12 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ //! This module provides types useful for static tests. #![allow(missing_docs, missing_debug_implementations)] -use crate::*; +use crate::{Builder, Error, Operation, ParseHttpResponse, ProvideErrorKind}; +use smithy_http::operation; #[derive(Debug)] #[non_exhaustive] @@ -27,7 +32,7 @@ pub struct TestOperation; impl ParseHttpResponse for TestOperation { type Output = Result<(), TestOperationError>; - fn parse_unloaded(&self, _: &mut http::Response) -> Option { + fn parse_unloaded(&self, _: &mut operation::Response) -> Option { unreachable!("only used for static tests") } @@ -51,7 +56,7 @@ fn sanity_retry() { // Statically check that a hyper client can actually be used to build a Client. #[allow(dead_code)] #[cfg(all(test, feature = "hyper"))] -fn sanity_hyper(hc: hyper::Client) +fn sanity_hyper(hc: hyper::Client) where C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, { diff --git a/rust-runtime/smithy-http-tower/src/dispatch.rs b/rust-runtime/smithy-http-tower/src/dispatch.rs index 481151eaab..d00f56ce6c 100644 --- a/rust-runtime/smithy-http-tower/src/dispatch.rs +++ b/rust-runtime/smithy-http-tower/src/dispatch.rs @@ -25,11 +25,11 @@ type BoxedResultFuture = Pin> + Send> impl Service for DispatchService where - S: Service> + Clone + Send + 'static, + S: Service, Response = http::Response> + Clone + Send + 'static, S::Error: Into, S::Future: Send + 'static, { - type Response = S::Response; + type Response = operation::Response; type Error = SendOperationError; type Future = BoxedResultFuture; @@ -40,13 +40,14 @@ where } fn call(&mut self, req: operation::Request) -> Self::Future { - let (req, _property_bag) = req.into_parts(); + let (req, property_bag) = req.into_parts(); let mut inner = self.inner.clone(); let future = async move { trace!(request = ?req); inner .call(req) .await + .map(|resp| operation::Response::from_parts(resp, property_bag)) .map_err(|e| SendOperationError::RequestDispatchError(e.into())) }; Box::pin(future) diff --git a/rust-runtime/smithy-http-tower/src/parse_response.rs b/rust-runtime/smithy-http-tower/src/parse_response.rs index 6bc04f87ae..1dce94a29c 100644 --- a/rust-runtime/smithy-http-tower/src/parse_response.rs +++ b/rust-runtime/smithy-http-tower/src/parse_response.rs @@ -4,7 +4,6 @@ */ use crate::SendOperationError; -use smithy_http::body::SdkBody; use smithy_http::middleware::load_response; use smithy_http::operation; use smithy_http::operation::Operation; @@ -46,7 +45,7 @@ impl ParseResponseLayer { impl Layer for ParseResponseLayer where - S: Service, + S: Service, { type Service = ParseResponseService; @@ -67,11 +66,10 @@ type BoxedResultFuture = Pin> + Send> /// `O`: The type of the response parser whose output type is `Result` /// `T`: The happy path return of the response parser /// `E`: The error path return of the response parser -/// `B`: The HTTP Body type returned by the inner service /// `R`: The type of the retry policy impl tower::Service> for ParseResponseService where - S: Service, Error = SendOperationError>, + S: Service, S::Future: Send + 'static, O: ParseHttpResponse> + Send + Sync + 'static, E: Error, @@ -107,7 +105,7 @@ where Err(e) => Err(e.into()), Ok(resp) => { // load_response contains reading the body as far as is required & parsing the response - let response_span = debug_span!("load_response",); + let response_span = debug_span!("load_response"); load_response(resp, &handler) .instrument(response_span) .await diff --git a/rust-runtime/smithy-http/src/middleware.rs b/rust-runtime/smithy-http/src/middleware.rs index 56489e2f09..f94c6971fe 100644 --- a/rust-runtime/smithy-http/src/middleware.rs +++ b/rust-runtime/smithy-http/src/middleware.rs @@ -14,7 +14,6 @@ use crate::pin_mut; use crate::response::ParseHttpResponse; use crate::result::{SdkError, SdkSuccess}; use bytes::{Buf, Bytes}; -use http::Response; use http_body::Body; use std::error::Error; use std::future::Future; @@ -82,11 +81,10 @@ pub trait MapRequest { /// /// 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, + mut response: operation::Response, handler: &O, ) -> Result, SdkError> where @@ -97,21 +95,28 @@ where return sdk_result(parsed_response, response); } - let (parts, body) = response.into_parts(); + let (http_response, properties) = response.into_parts(); + let (parts, body) = http_response.into_parts(); let body = match read_body(body).await { Ok(body) => body, Err(err) => { return Err(SdkError::ResponseError { - raw: Response::from_parts(parts, SdkBody::taken()), + raw: operation::Response::from_parts( + http::Response::from_parts(parts, SdkBody::taken()), + properties, + ), err, }); } }; - let response = Response::from_parts(parts, Bytes::from(body)); - trace!(response = ?response); - let parsed = handler.parse_loaded(&response); - sdk_result(parsed, response.map(SdkBody::from)) + let http_response = http::Response::from_parts(parts, Bytes::from(body)); + trace!(http_response = ?http_response); + let parsed = handler.parse_loaded(&http_response); + sdk_result( + parsed, + operation::Response::from_parts(http_response.map(SdkBody::from), properties), + ) } async fn read_body(body: B) -> Result, B::Error> { @@ -127,10 +132,10 @@ async fn read_body(body: B) -> Result, B::Error> { Ok(output) } -/// Convert a `Result` into an `SdkResult` that includes the raw HTTP response +/// Convert a `Result` into an `SdkResult` that includes the operation response fn sdk_result( parsed: Result, - raw: http::Response, + raw: operation::Response, ) -> Result, SdkError> { match parsed { Ok(parsed) => Ok(SdkSuccess { raw, parsed }), diff --git a/rust-runtime/smithy-http/src/operation.rs b/rust-runtime/smithy-http/src/operation.rs index cc0c23708f..76a8926173 100644 --- a/rust-runtime/smithy-http/src/operation.rs +++ b/rust-runtime/smithy-http/src/operation.rs @@ -81,12 +81,12 @@ impl Operation { Self { request, parts } } - pub fn config_mut(&mut self) -> impl DerefMut + '_ { - self.request.config_mut() + pub fn properties_mut(&mut self) -> impl DerefMut + '_ { + self.request.properties_mut() } - pub fn config(&self) -> impl Deref + '_ { - self.request.config() + pub fn properties(&self) -> impl Deref + '_ { + self.request.properties() } pub fn with_metadata(mut self, metadata: Metadata) -> Self { @@ -135,6 +135,9 @@ impl Operation { } } +/// Operation request type that associates a property bag with an underlying HTTP request. +/// This type represents the request in the Tower `Service` in middleware so that middleware +/// can share information with each other via the properties. #[derive(Debug)] pub struct Request { /// The underlying HTTP Request @@ -144,47 +147,56 @@ pub struct Request { /// /// Middleware can read and write from the property bag and use its /// contents to augment the request (see [`Request::augment`](Request::augment)) - configuration: Arc>, + properties: Arc>, } impl Request { - pub fn new(base: http::Request) -> Self { + /// Creates a new operation `Request` with the given `inner` HTTP request. + pub fn new(inner: http::Request) -> Self { Request { - inner: base, - configuration: Arc::new(Mutex::new(PropertyBag::new())), + inner, + properties: Arc::new(Mutex::new(PropertyBag::new())), } } + /// Allows modification of the HTTP request and associated properties with a fallible closure. pub fn augment( self, f: impl FnOnce(http::Request, &mut PropertyBag) -> Result, T>, ) -> Result { let inner = { - let configuration: &mut PropertyBag = &mut self.configuration.lock().unwrap(); - f(self.inner, configuration)? + let properties: &mut PropertyBag = &mut self.properties.lock().unwrap(); + f(self.inner, properties)? }; Ok(Request { inner, - configuration: self.configuration, + properties: self.properties, }) } - pub fn config_mut(&mut self) -> MutexGuard<'_, PropertyBag> { - self.configuration.lock().unwrap() + /// Gives mutable access to the properties. + pub fn properties_mut(&mut self) -> MutexGuard<'_, PropertyBag> { + self.properties.lock().unwrap() } - pub fn config(&self) -> MutexGuard<'_, PropertyBag> { - self.configuration.lock().unwrap() + /// Gives readonly access to the properties. + pub fn properties(&self) -> MutexGuard<'_, PropertyBag> { + self.properties.lock().unwrap() } - pub fn request_mut(&mut self) -> &mut http::Request { + /// Gives mutable access to the underlying HTTP request. + pub fn http_mut(&mut self) -> &mut http::Request { &mut self.inner } - pub fn request(&self) -> &http::Request { + /// Gives readonly access to the underlying HTTP request. + pub fn http(&self) -> &http::Request { &self.inner } + /// Attempts to clone the operation `Request`. This can fail if the + /// request body can't be cloned, such as if it is being streamed and the + /// stream can't be recreated. pub fn try_clone(&self) -> Option { let cloned_body = self.inner.body().try_clone()?; let mut cloned_request = http::Request::builder() @@ -199,12 +211,65 @@ impl Request { .expect("a clone of a valid request should be a valid request"); Some(Request { inner, - configuration: self.configuration.clone(), + properties: self.properties.clone(), }) } + /// Consumes the operation `Request` and returns the underlying HTTP request and properties. pub fn into_parts(self) -> (http::Request, Arc>) { - (self.inner, self.configuration) + (self.inner, self.properties) + } +} + +/// Operation response type that associates a property bag with an underlying HTTP response. +/// This type represents the response in the Tower `Service` in middleware so that middleware +/// can share information with each other via the properties. +#[derive(Debug)] +pub struct Response { + /// The underlying HTTP Response + inner: http::Response, + + /// Property bag of configuration options + properties: Arc>, +} + +impl Response { + /// Creates a new operation `Response` with the given `inner` HTTP response. + pub fn new(inner: http::Response) -> Self { + Response { + inner, + properties: Arc::new(Mutex::new(PropertyBag::new())), + } + } + + /// Gives mutable access to the properties. + pub fn properties_mut(&mut self) -> MutexGuard<'_, PropertyBag> { + self.properties.lock().unwrap() + } + + /// Gives readonly access to the properties. + pub fn properties(&self) -> MutexGuard<'_, PropertyBag> { + self.properties.lock().unwrap() + } + + /// Gives mutable access to the underlying HTTP response. + pub fn http_mut(&mut self) -> &mut http::Response { + &mut self.inner + } + + /// Gives readonly access to the underlying HTTP response. + pub fn http(&self) -> &http::Response { + &self.inner + } + + /// Consumes the operation `Request` and returns the underlying HTTP response and properties. + pub fn into_parts(self) -> (http::Response, Arc>) { + (self.inner, self.properties) + } + + /// Creates a new operation `Response` from an HTTP response and property bag. + pub fn from_parts(inner: http::Response, properties: Arc>) -> Self { + Response { inner, properties } } } @@ -226,7 +291,7 @@ mod test { .body(SdkBody::from("hello world!")) .expect("valid request"), ); - request.config_mut().insert("hello"); + request.properties_mut().insert("hello"); let cloned = request.try_clone().expect("request is cloneable"); let (request, config) = cloned.into_parts(); diff --git a/rust-runtime/smithy-http/src/response.rs b/rust-runtime/smithy-http/src/response.rs index 920ee026b2..d7902e7a53 100644 --- a/rust-runtime/smithy-http/src/response.rs +++ b/rust-runtime/smithy-http/src/response.rs @@ -3,9 +3,8 @@ * SPDX-License-Identifier: Apache-2.0. */ -use crate::body::SdkBody; +use crate::operation; use bytes::Bytes; -use http::Response; /// `ParseHttpResponse` is a generic trait for parsing structured data from HTTP responses. /// @@ -45,7 +44,7 @@ pub trait ParseHttpResponse { /// the streaming body with an empty body as long as the body implements default. /// /// We should consider if this is too limiting & if this should take an owned response instead. - fn parse_unloaded(&self, response: &mut http::Response) -> Option; + fn parse_unloaded(&self, response: &mut operation::Response) -> Option; /// Parse an HTTP request from a fully loaded body. This is for standard request/response style /// APIs like AwsJson 1.0/1.1 and the error path of most streaming APIs @@ -67,17 +66,17 @@ pub trait ParseHttpResponse { /// have cleaner implementations. There is a blanket implementation pub trait ParseStrictResponse { type Output; - fn parse(&self, response: &Response) -> Self::Output; + fn parse(&self, response: &http::Response) -> Self::Output; } impl ParseHttpResponse for T { type Output = T::Output; - fn parse_unloaded(&self, _response: &mut Response) -> Option { + fn parse_unloaded(&self, _response: &mut operation::Response) -> Option { None } - fn parse_loaded(&self, response: &Response) -> Self::Output { + fn parse_loaded(&self, response: &http::Response) -> Self::Output { self.parse(response) } } @@ -85,9 +84,9 @@ impl ParseHttpResponse for T { #[cfg(test)] mod test { use crate::body::SdkBody; + use crate::operation; use crate::response::ParseHttpResponse; use bytes::Bytes; - use http::Response; use std::mem; #[test] @@ -101,13 +100,13 @@ mod test { impl ParseHttpResponse for S3GetObjectParser { type Output = S3GetObject; - fn parse_unloaded(&self, response: &mut Response) -> Option { + fn parse_unloaded(&self, response: &mut operation::Response) -> Option { // For responses that pass on the body, use mem::take to leave behind an empty body - let body = mem::replace(response.body_mut(), SdkBody::taken()); + let body = mem::replace(response.http_mut().body_mut(), SdkBody::taken()); Some(S3GetObject { body }) } - fn parse_loaded(&self, _response: &Response) -> Self::Output { + fn parse_loaded(&self, _response: &http::Response) -> Self::Output { unimplemented!() } } diff --git a/rust-runtime/smithy-http/src/result.rs b/rust-runtime/smithy-http/src/result.rs index c337ff8c5f..5cda96d517 100644 --- a/rust-runtime/smithy-http/src/result.rs +++ b/rust-runtime/smithy-http/src/result.rs @@ -3,20 +3,21 @@ * SPDX-License-Identifier: Apache-2.0. */ -use crate::body::SdkBody; +use crate::operation; use std::error::Error; use std::fmt; use std::fmt::{Debug, Display, Formatter}; type BoxError = Box; -/// Successful Sdk Result + +/// Successful SDK Result #[derive(Debug)] pub struct SdkSuccess { - pub raw: http::Response, + pub raw: operation::Response, pub parsed: O, } -/// Failing Sdk Result +/// Failed SDK Result #[derive(Debug)] pub enum SdkError { /// The request failed during construction. It was not dispatched over the network. @@ -29,15 +30,12 @@ pub enum SdkError { /// 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, + raw: operation::Response, err: BoxError, }, /// An error response was received from the service - ServiceError { - err: E, - raw: http::Response, - }, + ServiceError { err: E, raw: operation::Response }, } impl Display for SdkError