From 5a8fff7a3222e519d977da5840a491e85ae65a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Sj=C3=B6=C3=B6h?= Date: Thu, 27 Apr 2023 18:16:01 +0200 Subject: [PATCH 01/10] implement `Ord` and `PartialOrd` for `DateTime` (#2653) ## Motivation and Context This change will allow easy sorting or comparing anything that contains a DateTime. My example is wanting to sort a list of S3 objects by last modified. This PR fixes #2406 ## Description It's a pretty small PR, it implements the `Ord` trait for `DateTime` by comparing the `seconds` property of `self` and `other`, if they are equal it compares the `subsec_nanos` properties instead. The `PartialOrd` trait is implemented by calling the `Ord` trait. ## Testing I added a unit test that compares a number of `DateTime` values with different combinations of positive/zero/negative `seconds`/`subsec_nanos`. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 6 +++ .../aws-smithy-types/src/date_time/mod.rs | 44 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index f0ae279af4..9f69d7e109 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -17,3 +17,9 @@ author = "rcoh" references = ["smithy-rs#2612"] meta = { "breaking" = false, "tada" = false, "bug" = false } + +[[smithy-rs]] +message = "Implement `Ord` and `PartialOrd` for `DateTime`." +author = "henriiik" +references = ["smithy-rs#2653"] +meta = { "breaking" = false, "tada" = false, "bug" = false } diff --git a/rust-runtime/aws-smithy-types/src/date_time/mod.rs b/rust-runtime/aws-smithy-types/src/date_time/mod.rs index 0459619998..ea3f5baf6c 100644 --- a/rust-runtime/aws-smithy-types/src/date_time/mod.rs +++ b/rust-runtime/aws-smithy-types/src/date_time/mod.rs @@ -9,6 +9,7 @@ use crate::date_time::format::rfc3339::AllowOffsets; use crate::date_time::format::DateTimeParseErrorKind; use num_integer::div_mod_floor; use num_integer::Integer; +use std::cmp::Ordering; use std::convert::TryFrom; use std::error::Error as StdError; use std::fmt; @@ -301,6 +302,21 @@ impl From for DateTime { } } +impl PartialOrd for DateTime { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for DateTime { + fn cmp(&self, other: &Self) -> Ordering { + match self.seconds.cmp(&other.seconds) { + Ordering::Equal => self.subsecond_nanos.cmp(&other.subsecond_nanos), + ordering => ordering, + } + } +} + /// Failure to convert a `DateTime` to or from another type. #[derive(Debug)] #[non_exhaustive] @@ -552,4 +568,32 @@ mod test { SystemTime::try_from(date_time).unwrap() ); } + + #[test] + fn ord() { + let first = DateTime::from_secs_and_nanos(-1, 0); + let second = DateTime::from_secs_and_nanos(0, 0); + let third = DateTime::from_secs_and_nanos(0, 1); + let fourth = DateTime::from_secs_and_nanos(1, 0); + + assert!(first == first); + assert!(first < second); + assert!(first < third); + assert!(first < fourth); + + assert!(second > first); + assert!(second == second); + assert!(second < third); + assert!(second < fourth); + + assert!(third > first); + assert!(third > second); + assert!(third == third); + assert!(third < fourth); + + assert!(fourth > first); + assert!(fourth > second); + assert!(fourth > third); + assert!(fourth == fourth); + } } From b1ce5e3c6c1b6f0f2538ebfc3a764d05b62529c2 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 27 Apr 2023 10:26:33 -0700 Subject: [PATCH 02/10] Recalculate signing time for every retry attempt (#2643) ## Motivation and Context When SigV4 signing was ported over to the orchestrator, the request time got calculated once before the retry loop, which was incorrect. This PR moves that request time calculation into the request signer so that it happens on every attempt (unless there is a configured request time override). This PR also refactors auth to use the `ConfigBag` instead of a separate signing properties `PropertyBag`. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- aws/rust-runtime/aws-runtime/src/auth.rs | 34 +-- aws/rust-runtime/aws-runtime/src/identity.rs | 4 +- .../smithy/rustsdk/SigV4AuthDecorator.kt | 13 +- .../benches/middleware_vs_orchestrator.rs | 256 +++--------------- .../aws-sdk-s3/tests/sra_test.rs | 4 - .../customizations/HttpAuthDecorator.kt | 38 +-- .../OperationRuntimePluginGenerator.kt | 6 +- .../ServiceRuntimePluginGenerator.kt | 6 +- .../aws-smithy-runtime-api/src/client/auth.rs | 134 +++++++++ .../src/client/auth/http.rs | 10 +- .../src/client/auth/option_resolver.rs | 27 +- .../src/client/identity.rs | 20 +- .../src/client/identity/http.rs | 6 +- .../src/client/orchestrator.rs | 137 +--------- .../src/client/auth/http.rs | 39 ++- .../src/client/orchestrator/auth.rs | 45 ++- 16 files changed, 280 insertions(+), 499 deletions(-) diff --git a/aws/rust-runtime/aws-runtime/src/auth.rs b/aws/rust-runtime/aws-runtime/src/auth.rs index cf6f3b9ac3..1da617abdd 100644 --- a/aws/rust-runtime/aws-runtime/src/auth.rs +++ b/aws/rust-runtime/aws-runtime/src/auth.rs @@ -11,11 +11,10 @@ pub mod sigv4 { SignableRequest, SignatureLocation, SigningParams, SigningSettings, UriPathNormalizationMode, }; - use aws_smithy_http::property_bag::PropertyBag; + use aws_smithy_runtime_api::client::auth::{AuthSchemeId, HttpAuthScheme, HttpRequestSigner}; use aws_smithy_runtime_api::client::identity::{Identity, IdentityResolver, IdentityResolvers}; - use aws_smithy_runtime_api::client::orchestrator::{ - BoxError, HttpAuthScheme, HttpRequest, HttpRequestSigner, - }; + use aws_smithy_runtime_api::client::orchestrator::{BoxError, ConfigBagAccessors, HttpRequest}; + use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_types::region::SigningRegion; use aws_types::SigningService; use std::time::{Duration, SystemTime}; @@ -24,7 +23,7 @@ pub mod sigv4 { `expires_in` duration because the credentials used to sign it will expire first."; /// Auth scheme ID for SigV4. - pub const SCHEME_ID: &str = "sigv4"; + pub const SCHEME_ID: AuthSchemeId = AuthSchemeId::new("sigv4"); /// SigV4 auth scheme. #[derive(Debug, Default)] @@ -40,7 +39,7 @@ pub mod sigv4 { } impl HttpAuthScheme for SigV4HttpAuthScheme { - fn scheme_id(&self) -> &'static str { + fn scheme_id(&self) -> AuthSchemeId { SCHEME_ID } @@ -88,8 +87,6 @@ pub mod sigv4 { pub signing_optional: bool, /// Optional expiration (for presigning) pub expires_in: Option, - /// Timestamp to sign with. - pub request_timestamp: SystemTime, } impl Default for SigningOptions { @@ -103,7 +100,6 @@ pub mod sigv4 { signature_type: HttpSignatureType::HttpRequestHeaders, signing_optional: false, expires_in: None, - request_timestamp: SystemTime::now(), } } } @@ -168,11 +164,11 @@ pub mod sigv4 { settings: SigningSettings, credentials: &'a Credentials, operation_config: &'a SigV4OperationSigningConfig, + request_timestamp: SystemTime, ) -> SigningParams<'a> { if let Some(expires_in) = settings.expires_in { if let Some(creds_expires_time) = credentials.expiry() { - let presigned_expires_time = - operation_config.signing_options.request_timestamp + expires_in; + let presigned_expires_time = request_timestamp + expires_in; if presigned_expires_time > creds_expires_time { tracing::warn!(EXPIRATION_WARNING); } @@ -184,7 +180,7 @@ pub mod sigv4 { .secret_key(credentials.secret_access_key()) .region(operation_config.region.as_ref()) .service_name(operation_config.service.as_ref()) - .time(operation_config.signing_options.request_timestamp) + .time(request_timestamp) .settings(settings); builder.set_security_token(credentials.session_token()); builder.build().expect("all required fields set") @@ -196,12 +192,12 @@ pub mod sigv4 { &self, request: &mut HttpRequest, identity: &Identity, - // TODO(enableNewSmithyRuntime): should this be the config bag? - signing_properties: &PropertyBag, + config_bag: &ConfigBag, ) -> Result<(), BoxError> { - let operation_config = signing_properties + let operation_config = config_bag .get::() .ok_or("missing operation signing config for SigV4")?; + let request_time = config_bag.request_time().unwrap_or_default().system_time(); let credentials = if let Some(creds) = identity.data::() { creds @@ -213,7 +209,8 @@ pub mod sigv4 { }; let settings = Self::settings(operation_config); - let signing_params = Self::signing_params(settings, credentials, operation_config); + let signing_params = + Self::signing_params(settings, credentials, operation_config, request_time); let (signing_instructions, _signature) = { // A body that is already in memory can be signed directly. A body that is not in memory @@ -283,17 +280,16 @@ pub mod sigv4 { signature_type: HttpSignatureType::HttpRequestHeaders, signing_optional: false, expires_in: None, - request_timestamp: now, payload_override: None, }, }; - SigV4HttpRequestSigner::signing_params(settings, &credentials, &operation_config); + SigV4HttpRequestSigner::signing_params(settings, &credentials, &operation_config, now); assert!(!logs_contain(EXPIRATION_WARNING)); let mut settings = SigningSettings::default(); settings.expires_in = Some(creds_expire_in + Duration::from_secs(10)); - SigV4HttpRequestSigner::signing_params(settings, &credentials, &operation_config); + SigV4HttpRequestSigner::signing_params(settings, &credentials, &operation_config, now); assert!(logs_contain(EXPIRATION_WARNING)); } } diff --git a/aws/rust-runtime/aws-runtime/src/identity.rs b/aws/rust-runtime/aws-runtime/src/identity.rs index 2e2eff1bf2..c120e4eb7f 100644 --- a/aws/rust-runtime/aws-runtime/src/identity.rs +++ b/aws/rust-runtime/aws-runtime/src/identity.rs @@ -6,9 +6,9 @@ /// Credentials-based identity support. pub mod credentials { use aws_credential_types::cache::SharedCredentialsCache; - use aws_smithy_http::property_bag::PropertyBag; use aws_smithy_runtime_api::client::identity::{Identity, IdentityResolver}; use aws_smithy_runtime_api::client::orchestrator::{BoxError, Future}; + use aws_smithy_runtime_api::config_bag::ConfigBag; /// Smithy identity resolver for AWS credentials. #[derive(Debug)] @@ -24,7 +24,7 @@ pub mod credentials { } impl IdentityResolver for CredentialsIdentityResolver { - fn resolve_identity(&self, _identity_properties: &PropertyBag) -> Future { + fn resolve_identity(&self, _config_bag: &ConfigBag) -> Future { let cache = self.credentials_cache.clone(); Future::new(Box::pin(async move { let credentials = cache.as_ref().provide_cached_credentials().await?; diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt index 7ae7582e69..f4973330c5 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt @@ -102,10 +102,8 @@ private class AuthOperationRuntimePluginCustomization(private val codegenContext val runtimeApi = RuntimeType.smithyRuntimeApi(runtimeConfig) val awsRuntime = AwsRuntimeType.awsRuntime(runtimeConfig) arrayOf( - "AuthOptionListResolver" to runtimeApi.resolve("client::auth::option_resolver::AuthOptionListResolver"), - "HttpAuthOption" to runtimeApi.resolve("client::orchestrator::HttpAuthOption"), + "StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "HttpSignatureType" to awsRuntime.resolve("auth::sigv4::HttpSignatureType"), - "PropertyBag" to RuntimeType.smithyHttp(runtimeConfig).resolve("property_bag::PropertyBag"), "SIGV4_SCHEME_ID" to awsRuntime.resolve("auth::sigv4::SCHEME_ID"), "SigV4OperationSigningConfig" to awsRuntime.resolve("auth::sigv4::SigV4OperationSigningConfig"), "SigningOptions" to awsRuntime.resolve("auth::sigv4::SigningOptions"), @@ -136,16 +134,15 @@ private class AuthOperationRuntimePluginCustomization(private val codegenContext signing_options.normalize_uri_path = $normalizeUrlPath; signing_options.signing_optional = $signingOptional; signing_options.payload_override = #{payload_override}; - signing_options.request_timestamp = cfg.request_time().unwrap_or_default().system_time(); - let mut sigv4_properties = #{PropertyBag}::new(); - sigv4_properties.insert(#{SigV4OperationSigningConfig} { + ${section.configBagName}.put(#{SigV4OperationSigningConfig} { region: signing_region, service: signing_service, signing_options, }); - let auth_option_resolver = #{AuthOptionListResolver}::new( - vec![#{HttpAuthOption}::new(#{SIGV4_SCHEME_ID}, std::sync::Arc::new(sigv4_properties))] + // TODO(enableNewSmithyRuntime): Make auth options additive in the config bag so that multiple codegen decorators can register them + let auth_option_resolver = #{StaticAuthOptionResolver}::new( + vec![#{SIGV4_SCHEME_ID}] ); ${section.configBagName}.set_auth_option_resolver(auth_option_resolver); """, diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs index d587cdb0b4..3fba943ba1 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -5,16 +5,13 @@ #[macro_use] extern crate criterion; -use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache}; -use aws_credential_types::provider::SharedCredentialsProvider; -use aws_credential_types::Credentials; use aws_sdk_s3 as s3; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::test_connection::infallible_connection_fn; -use aws_smithy_http::endpoint::SharedEndpointResolver; -use aws_smithy_runtime_api::type_erasure::TypedBox; +use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; +use aws_smithy_runtime_api::config_bag::ConfigBag; use criterion::Criterion; -use s3::operation::list_objects_v2::{ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output}; +use s3::endpoint::Params; async fn middleware(client: &s3::Client) { client @@ -26,41 +23,36 @@ async fn middleware(client: &s3::Client) { .expect("successful execution"); } -async fn orchestrator( - connector: &DynConnector, - endpoint_resolver: SharedEndpointResolver, - credentials_cache: SharedCredentialsCache, -) { - let service_runtime_plugin = orchestrator::ManualServiceRuntimePlugin { - connector: connector.clone(), - endpoint_resolver: endpoint_resolver.clone(), - credentials_cache: credentials_cache.clone(), - }; +async fn orchestrator(client: &s3::Client) { + struct FixupPlugin { + region: String, + } + impl RuntimePlugin for FixupPlugin { + fn configure( + &self, + cfg: &mut ConfigBag, + ) -> Result<(), aws_smithy_runtime_api::client::runtime_plugin::BoxError> { + let params_builder = Params::builder() + .set_region(Some(self.region.clone())) + .bucket("test-bucket"); - // TODO(enableNewSmithyRuntime): benchmark with `send_v2` directly once it works - let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new() - .with_client_plugin(service_runtime_plugin) - .with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new()) - .with_operation_plugin(orchestrator::ManualOperationRuntimePlugin); - let input = ListObjectsV2Input::builder() + cfg.put(params_builder); + Ok(()) + } + } + let _output = client + .list_objects_v2() .bucket("test-bucket") .prefix("prefix~") - .build() - .unwrap(); - let input = TypedBox::new(input).erase(); - let output = aws_smithy_runtime::client::orchestrator::invoke(input, &runtime_plugins) + .send_v2_with_plugin(Some(FixupPlugin { + region: client + .conf() + .region() + .map(|c| c.as_ref().to_string()) + .unwrap(), + })) .await - .map_err(|err| { - err.map_service_error(|err| { - TypedBox::::assume_from(err) - .expect("correct error type") - .unwrap() - }) - }) - .unwrap(); - TypedBox::::assume_from(output) - .expect("correct output type") - .unwrap(); + .expect("successful execution"); } fn test_connection() -> DynConnector { @@ -93,14 +85,18 @@ fn test_connection() -> DynConnector { }) } -fn middleware_bench(c: &mut Criterion) { +fn client() -> s3::Client { let conn = test_connection(); let config = s3::Config::builder() .credentials_provider(s3::config::Credentials::for_tests()) .region(s3::config::Region::new("us-east-1")) .http_connector(conn.clone()) .build(); - let client = s3::Client::from_conf(config); + s3::Client::from_conf(config) +} + +fn middleware_bench(c: &mut Criterion) { + let client = client(); c.bench_function("middleware", move |b| { b.to_async(tokio::runtime::Runtime::new().unwrap()) .iter(|| async { middleware(&client).await }) @@ -108,190 +104,12 @@ fn middleware_bench(c: &mut Criterion) { } fn orchestrator_bench(c: &mut Criterion) { - let conn = test_connection(); - let endpoint_resolver = SharedEndpointResolver::new(s3::endpoint::DefaultResolver::new()); - let credentials_cache = SharedCredentialsCache::new( - CredentialsCache::lazy() - .create_cache(SharedCredentialsProvider::new(Credentials::for_tests())), - ); - + let client = client(); c.bench_function("orchestrator", move |b| { b.to_async(tokio::runtime::Runtime::new().unwrap()) - .iter(|| async { - orchestrator(&conn, endpoint_resolver.clone(), credentials_cache.clone()).await - }) + .iter(|| async { orchestrator(&client).await }) }); } -mod orchestrator { - use aws_credential_types::cache::SharedCredentialsCache; - use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; - use aws_runtime::recursion_detection::RecursionDetectionInterceptor; - use aws_runtime::user_agent::UserAgentInterceptor; - use aws_sdk_s3::config::Region; - use aws_sdk_s3::endpoint::Params; - use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Input; - use aws_smithy_client::erase::DynConnector; - use aws_smithy_http::endpoint::SharedEndpointResolver; - use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter; - use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver; - use aws_smithy_runtime_api::client::interceptors::error::ContextAttachedError; - use aws_smithy_runtime_api::client::interceptors::{ - Interceptor, InterceptorContext, Interceptors, - }; - use aws_smithy_runtime_api::client::orchestrator::{ - BoxError, ConfigBagAccessors, Connection, HttpRequest, HttpResponse, TraceProbe, - }; - use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; - use aws_smithy_runtime_api::config_bag::ConfigBag; - use aws_types::region::SigningRegion; - use aws_types::SigningService; - use std::sync::Arc; - - pub struct ManualServiceRuntimePlugin { - pub connector: DynConnector, - pub endpoint_resolver: SharedEndpointResolver, - pub credentials_cache: SharedCredentialsCache, - } - - impl RuntimePlugin for ManualServiceRuntimePlugin { - fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { - let identity_resolvers = - aws_smithy_runtime_api::client::identity::IdentityResolvers::builder() - .identity_resolver( - aws_runtime::auth::sigv4::SCHEME_ID, - aws_runtime::identity::credentials::CredentialsIdentityResolver::new( - self.credentials_cache.clone(), - ), - ) - .identity_resolver( - "anonymous", - aws_smithy_runtime_api::client::identity::AnonymousIdentityResolver::new(), - ) - .build(); - cfg.set_identity_resolvers(identity_resolvers); - - let http_auth_schemes = - aws_smithy_runtime_api::client::orchestrator::HttpAuthSchemes::builder() - .auth_scheme( - aws_runtime::auth::sigv4::SCHEME_ID, - aws_runtime::auth::sigv4::SigV4HttpAuthScheme::new(), - ) - .build(); - cfg.set_http_auth_schemes(http_auth_schemes); - - cfg.set_auth_option_resolver( - aws_smithy_runtime_api::client::auth::option_resolver::AuthOptionListResolver::new( - Vec::new(), - ), - ); - - cfg.set_endpoint_resolver(DefaultEndpointResolver::new(self.endpoint_resolver.clone())); - - let params_builder = aws_sdk_s3::endpoint::Params::builder() - .set_region(Some("us-east-1".to_owned())) - .set_endpoint(Some("https://s3.us-east-1.amazonaws.com/".to_owned())); - cfg.put(params_builder); - - cfg.set_retry_strategy( - aws_smithy_runtime_api::client::retries::NeverRetryStrategy::new(), - ); - - let connection: Box = - Box::new(DynConnectorAdapter::new(self.connector.clone())); - cfg.set_connection(connection); - - cfg.set_trace_probe({ - #[derive(Debug)] - struct StubTraceProbe; - impl TraceProbe for StubTraceProbe { - fn dispatch_events(&self) { - // no-op - } - } - StubTraceProbe - }); - - cfg.put(SigningService::from_static("s3")); - cfg.put(SigningRegion::from(Region::from_static("us-east-1"))); - - cfg.put(ApiMetadata::new("unused", "unused")); - cfg.put(AwsUserAgent::for_tests()); // Override the user agent with the test UA - cfg.get::>() - .expect("interceptors set") - .register_client_interceptor(Arc::new(UserAgentInterceptor::new()) as _) - .register_client_interceptor(Arc::new(RecursionDetectionInterceptor::new()) as _); - Ok(()) - } - } - - // This is a temporary operation runtime plugin until EndpointParamsInterceptor and - // EndpointParamsFinalizerInterceptor have been fully implemented, in which case - // `.with_operation_plugin(ManualOperationRuntimePlugin)` can be removed. - pub struct ManualOperationRuntimePlugin; - - impl RuntimePlugin for ManualOperationRuntimePlugin { - fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { - #[derive(Debug)] - struct ListObjectsV2EndpointParamsInterceptor; - impl Interceptor for ListObjectsV2EndpointParamsInterceptor { - fn read_before_execution( - &self, - context: &InterceptorContext, - cfg: &mut ConfigBag, - ) -> Result<(), BoxError> { - let input = context.input()?; - let input = input - .downcast_ref::() - .ok_or_else(|| "failed to downcast to ListObjectsV2Input")?; - let mut params_builder = cfg - .get::() - .ok_or_else(|| "missing endpoint params builder")? - .clone(); - params_builder = params_builder.set_bucket(input.bucket.clone()); - cfg.put(params_builder); - - Ok(()) - } - } - - #[derive(Debug)] - struct ListObjectsV2EndpointParamsFinalizerInterceptor; - impl Interceptor for ListObjectsV2EndpointParamsFinalizerInterceptor { - fn read_before_execution( - &self, - _context: &InterceptorContext, - cfg: &mut ConfigBag, - ) -> Result<(), BoxError> { - let params_builder = cfg - .get::() - .ok_or_else(|| "missing endpoint params builder")? - .clone(); - let params = params_builder.build().map_err(|err| { - ContextAttachedError::new("endpoint params could not be built", err) - })?; - cfg.put( - aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new( - params, - ), - ); - - Ok(()) - } - } - - cfg.get::>() - .expect("interceptors set") - .register_operation_interceptor( - Arc::new(ListObjectsV2EndpointParamsInterceptor) as _ - ) - .register_operation_interceptor(Arc::new( - ListObjectsV2EndpointParamsFinalizerInterceptor, - ) as _); - Ok(()) - } - } -} - criterion_group!(benches, middleware_bench, orchestrator_bench); criterion_main!(benches); diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index b306f42753..464c815d68 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -7,17 +7,13 @@ use aws_http::user_agent::AwsUserAgent; use aws_runtime::invocation_id::InvocationId; use aws_sdk_s3::config::{Credentials, Region}; use aws_sdk_s3::endpoint::Params; - use aws_sdk_s3::Client; - use aws_smithy_client::dvr; use aws_smithy_client::dvr::MediaType; use aws_smithy_client::erase::DynConnector; - use aws_smithy_runtime_api::client::orchestrator::{ConfigBagAccessors, RequestTime}; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; use aws_smithy_runtime_api::config_bag::ConfigBag; - use std::time::{Duration, SystemTime, UNIX_EPOCH}; const LIST_BUCKETS_PATH: &str = "test-data/list-objects-v2.json"; diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt index f9757b4ef9..53aa5ee2aa 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt @@ -43,7 +43,7 @@ fun codegenScope(runtimeConfig: RuntimeConfig): Array> { return arrayOf( "ApiKeyAuthScheme" to authHttp.resolve("ApiKeyAuthScheme"), "ApiKeyLocation" to authHttp.resolve("ApiKeyLocation"), - "AuthOptionListResolver" to smithyRuntimeApi.resolve("client::auth::option_resolver::AuthOptionListResolver"), + "StaticAuthOptionResolver" to smithyRuntimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "BasicAuthScheme" to authHttp.resolve("BasicAuthScheme"), "BearerAuthScheme" to authHttp.resolve("BearerAuthScheme"), "DigestAuthScheme" to authHttp.resolve("DigestAuthScheme"), @@ -51,7 +51,6 @@ fun codegenScope(runtimeConfig: RuntimeConfig): Array> { "HTTP_BASIC_AUTH_SCHEME_ID" to authHttpApi.resolve("HTTP_BASIC_AUTH_SCHEME_ID"), "HTTP_BEARER_AUTH_SCHEME_ID" to authHttpApi.resolve("HTTP_BEARER_AUTH_SCHEME_ID"), "HTTP_DIGEST_AUTH_SCHEME_ID" to authHttpApi.resolve("HTTP_DIGEST_AUTH_SCHEME_ID"), - "HttpAuthOption" to smithyRuntimeApi.resolve("client::orchestrator::HttpAuthOption"), "IdentityResolver" to smithyRuntimeApi.resolve("client::identity::IdentityResolver"), "IdentityResolvers" to smithyRuntimeApi.resolve("client::identity::IdentityResolvers"), "Login" to smithyRuntimeApi.resolve("client::identity::http::Login"), @@ -208,46 +207,23 @@ private class HttpAuthOperationRuntimePluginCustomization( when (section) { is OperationRuntimePluginSection.AdditionalConfig -> { withBlockTemplate( - "let auth_option_resolver = #{AuthOptionListResolver}::new(vec![", + "let auth_option_resolver = #{StaticAuthOptionResolver}::new(vec![", "]);", *codegenScope, ) { val authTrait: AuthTrait? = section.operationShape.getTrait() ?: serviceShape.getTrait() for (authScheme in authTrait?.valueSet ?: emptySet()) { when (authScheme) { - HttpApiKeyAuthTrait.ID -> { - rustTemplate( - "#{HttpAuthOption}::new(#{HTTP_API_KEY_AUTH_SCHEME_ID}, std::sync::Arc::new(#{PropertyBag}::new())),", - *codegenScope, - ) - } - - HttpBasicAuthTrait.ID -> { - rustTemplate( - "#{HttpAuthOption}::new(#{HTTP_BASIC_AUTH_SCHEME_ID}, std::sync::Arc::new(#{PropertyBag}::new())),", - *codegenScope, - ) - } - - HttpBearerAuthTrait.ID -> { - rustTemplate( - "#{HttpAuthOption}::new(#{HTTP_BEARER_AUTH_SCHEME_ID}, std::sync::Arc::new(#{PropertyBag}::new())),", - *codegenScope, - ) - } - - HttpDigestAuthTrait.ID -> { - rustTemplate( - "#{HttpAuthOption}::new(#{HTTP_DIGEST_AUTH_SCHEME_ID}, std::sync::Arc::new(#{PropertyBag}::new())),", - *codegenScope, - ) - } - + HttpApiKeyAuthTrait.ID -> rustTemplate("#{HTTP_API_KEY_AUTH_SCHEME_ID},", *codegenScope) + HttpBasicAuthTrait.ID -> rustTemplate("#{HTTP_BASIC_AUTH_SCHEME_ID},", *codegenScope) + HttpBearerAuthTrait.ID -> rustTemplate("#{HTTP_BEARER_AUTH_SCHEME_ID},", *codegenScope) + HttpDigestAuthTrait.ID -> rustTemplate("#{HTTP_DIGEST_AUTH_SCHEME_ID},", *codegenScope) else -> {} } } } + // TODO(enableNewSmithyRuntime): Make auth options additive in the config bag so that multiple codegen decorators can register them rustTemplate("${section.configBagName}.set_auth_option_resolver(auth_option_resolver);", *codegenScope) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt index 7c2d1e7662..a2ba50cb6f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt @@ -76,8 +76,8 @@ class OperationRuntimePluginGenerator( private val codegenScope = codegenContext.runtimeConfig.let { rc -> val runtimeApi = RuntimeType.smithyRuntimeApi(rc) arrayOf( - "AuthOptionListResolverParams" to runtimeApi.resolve("client::auth::option_resolver::AuthOptionListResolverParams"), - "AuthOptionResolverParams" to runtimeApi.resolve("client::orchestrator::AuthOptionResolverParams"), + "StaticAuthOptionResolverParams" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolverParams"), + "AuthOptionResolverParams" to runtimeApi.resolve("client::auth::AuthOptionResolverParams"), "BoxError" to runtimeApi.resolve("client::runtime_plugin::BoxError"), "ConfigBag" to runtimeApi.resolve("config_bag::ConfigBag"), "ConfigBagAccessors" to runtimeApi.resolve("client::orchestrator::ConfigBagAccessors"), @@ -101,7 +101,7 @@ class OperationRuntimePluginGenerator( cfg.set_response_deserializer(${operationStructName}ResponseDeserializer); ${"" /* TODO(IdentityAndAuth): Resolve auth parameters from input for services that need this */} - cfg.set_auth_option_resolver_params(#{AuthOptionResolverParams}::new(#{AuthOptionListResolverParams}::new())); + cfg.set_auth_option_resolver_params(#{AuthOptionResolverParams}::new(#{StaticAuthOptionResolverParams}::new())); // Retry classifiers are operation-specific because they need to downcast operation-specific error types. let retry_classifiers = #{RetryClassifiers}::new() diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index 7f5963c572..a570d4df91 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -70,7 +70,7 @@ class ServiceRuntimePluginGenerator( val runtimeApi = RuntimeType.smithyRuntimeApi(rc) arrayOf( "AnonymousIdentityResolver" to runtimeApi.resolve("client::identity::AnonymousIdentityResolver"), - "AuthOptionListResolver" to runtimeApi.resolve("client::auth::option_resolver::AuthOptionListResolver"), + "StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "BoxError" to runtimeApi.resolve("client::runtime_plugin::BoxError"), "ConfigBag" to runtimeApi.resolve("config_bag::ConfigBag"), "ConfigBagAccessors" to runtimeApi.resolve("client::orchestrator::ConfigBagAccessors"), @@ -78,7 +78,7 @@ class ServiceRuntimePluginGenerator( "ConnectorSettings" to RuntimeType.smithyClient(rc).resolve("http_connector::ConnectorSettings"), "DefaultEndpointResolver" to runtime.resolve("client::orchestrator::endpoints::DefaultEndpointResolver"), "DynConnectorAdapter" to runtime.resolve("client::connections::adapter::DynConnectorAdapter"), - "HttpAuthSchemes" to runtimeApi.resolve("client::orchestrator::HttpAuthSchemes"), + "HttpAuthSchemes" to runtimeApi.resolve("client::auth::HttpAuthSchemes"), "IdentityResolvers" to runtimeApi.resolve("client::identity::IdentityResolvers"), "NeverRetryStrategy" to runtime.resolve("client::retries::strategy::NeverRetryStrategy"), "Params" to endpointTypesGenerator.paramsStruct(), @@ -112,7 +112,7 @@ class ServiceRuntimePluginGenerator( cfg.set_http_auth_schemes(http_auth_schemes); // Set an empty auth option resolver to be overridden by operations that need auth. - cfg.set_auth_option_resolver(#{AuthOptionListResolver}::new(Vec::new())); + cfg.set_auth_option_resolver(#{StaticAuthOptionResolver}::new(Vec::new())); let endpoint_resolver = #{DefaultEndpointResolver}::<#{Params}>::new( #{SharedEndpointResolver}::from(self.handle.conf.endpoint_resolver())); diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs b/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs index 0438c1bdfa..3d8cccad47 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs @@ -3,7 +3,141 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::client::identity::{Identity, IdentityResolver, IdentityResolvers}; +use crate::client::orchestrator::{BoxError, HttpRequest}; +use crate::config_bag::ConfigBag; +use crate::type_erasure::{TypeErasedBox, TypedBox}; +use std::any::Any; +use std::borrow::Cow; +use std::fmt::Debug; +use std::sync::Arc; + #[cfg(feature = "http-auth")] pub mod http; pub mod option_resolver; + +/// New type around an auth scheme ID. +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] +pub struct AuthSchemeId { + scheme_id: &'static str, +} + +impl AuthSchemeId { + /// Creates a new auth scheme ID. + pub const fn new(scheme_id: &'static str) -> Self { + Self { scheme_id } + } + + /// Returns the string equivalent of this auth scheme ID. + pub const fn as_str(&self) -> &'static str { + self.scheme_id + } +} + +#[derive(Debug)] +pub struct AuthOptionResolverParams(TypeErasedBox); + +impl AuthOptionResolverParams { + pub fn new(params: T) -> Self { + Self(TypedBox::new(params).erase()) + } + + pub fn get(&self) -> Option<&T> { + self.0.downcast_ref() + } +} + +pub trait AuthOptionResolver: Send + Sync + Debug { + fn resolve_auth_options<'a>( + &'a self, + params: &AuthOptionResolverParams, + ) -> Result, BoxError>; +} + +impl AuthOptionResolver for Box { + fn resolve_auth_options<'a>( + &'a self, + params: &AuthOptionResolverParams, + ) -> Result, BoxError> { + (**self).resolve_auth_options(params) + } +} + +#[derive(Debug)] +struct HttpAuthSchemesInner { + schemes: Vec<(AuthSchemeId, Box)>, +} +#[derive(Debug)] +pub struct HttpAuthSchemes { + inner: Arc, +} + +impl HttpAuthSchemes { + pub fn builder() -> builders::HttpAuthSchemesBuilder { + Default::default() + } + + pub fn scheme(&self, scheme_id: AuthSchemeId) -> Option<&dyn HttpAuthScheme> { + self.inner + .schemes + .iter() + .find(|scheme| scheme.0 == scheme_id) + .map(|scheme| &*scheme.1) + } +} + +pub trait HttpAuthScheme: Send + Sync + Debug { + fn scheme_id(&self) -> AuthSchemeId; + + fn identity_resolver<'a>( + &self, + identity_resolvers: &'a IdentityResolvers, + ) -> Option<&'a dyn IdentityResolver>; + + fn request_signer(&self) -> &dyn HttpRequestSigner; +} + +pub trait HttpRequestSigner: Send + Sync + Debug { + /// Return a signed version of the given request using the given identity. + /// + /// If the provided identity is incompatible with this signer, an error must be returned. + fn sign_request( + &self, + request: &mut HttpRequest, + identity: &Identity, + config_bag: &ConfigBag, + ) -> Result<(), BoxError>; +} + +pub mod builders { + use super::*; + + #[derive(Debug, Default)] + pub struct HttpAuthSchemesBuilder { + schemes: Vec<(AuthSchemeId, Box)>, + } + + impl HttpAuthSchemesBuilder { + pub fn new() -> Self { + Default::default() + } + + pub fn auth_scheme( + mut self, + scheme_id: AuthSchemeId, + auth_scheme: impl HttpAuthScheme + 'static, + ) -> Self { + self.schemes.push((scheme_id, Box::new(auth_scheme) as _)); + self + } + + pub fn build(self) -> HttpAuthSchemes { + HttpAuthSchemes { + inner: Arc::new(HttpAuthSchemesInner { + schemes: self.schemes, + }), + } + } + } +} diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/auth/http.rs b/rust-runtime/aws-smithy-runtime-api/src/client/auth/http.rs index 2f93a2eb40..a4583c6e0a 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/auth/http.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/auth/http.rs @@ -3,7 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -pub const HTTP_API_KEY_AUTH_SCHEME_ID: &str = "http-api-key-auth"; -pub const HTTP_BASIC_AUTH_SCHEME_ID: &str = "http-basic-auth"; -pub const HTTP_BEARER_AUTH_SCHEME_ID: &str = "http-bearer-auth"; -pub const HTTP_DIGEST_AUTH_SCHEME_ID: &str = "http-digest-auth"; +use crate::client::auth::AuthSchemeId; + +pub const HTTP_API_KEY_AUTH_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("http-api-key-auth"); +pub const HTTP_BASIC_AUTH_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("http-basic-auth"); +pub const HTTP_BEARER_AUTH_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("http-bearer-auth"); +pub const HTTP_DIGEST_AUTH_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("http-digest-auth"); diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/auth/option_resolver.rs b/rust-runtime/aws-smithy-runtime-api/src/client/auth/option_resolver.rs index ca3acef5f6..851233486b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/auth/option_resolver.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/auth/option_resolver.rs @@ -3,41 +3,40 @@ * SPDX-License-Identifier: Apache-2.0 */ -use crate::client::orchestrator::{ - AuthOptionResolver, AuthOptionResolverParams, BoxError, HttpAuthOption, -}; +use crate::client::auth::{AuthOptionResolver, AuthOptionResolverParams, AuthSchemeId}; +use crate::client::orchestrator::BoxError; use std::borrow::Cow; /// New-type around a `Vec` that implements `AuthOptionResolver`. /// /// This is useful for clients that don't require `AuthOptionResolverParams` to resolve auth options. #[derive(Debug)] -pub struct AuthOptionListResolver { - auth_options: Vec, +pub struct StaticAuthOptionResolver { + auth_options: Vec, } -impl AuthOptionListResolver { - /// Creates a new instance of `AuthOptionListResolver`. - pub fn new(auth_options: Vec) -> Self { +impl StaticAuthOptionResolver { + /// Creates a new instance of `StaticAuthOptionResolver`. + pub fn new(auth_options: Vec) -> Self { Self { auth_options } } } -impl AuthOptionResolver for AuthOptionListResolver { +impl AuthOptionResolver for StaticAuthOptionResolver { fn resolve_auth_options<'a>( &'a self, _params: &AuthOptionResolverParams, - ) -> Result, BoxError> { + ) -> Result, BoxError> { Ok(Cow::Borrowed(&self.auth_options)) } } -/// Empty params to be used with [`AuthOptionListResolver`]. +/// Empty params to be used with [`StaticAuthOptionResolver`]. #[derive(Debug)] -pub struct AuthOptionListResolverParams; +pub struct StaticAuthOptionResolverParams; -impl AuthOptionListResolverParams { - /// Creates new `AuthOptionListResolverParams`. +impl StaticAuthOptionResolverParams { + /// Creates new `StaticAuthOptionResolverParams`. pub fn new() -> Self { Self } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs index 11a54af820..6d5cf9ff82 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs @@ -3,8 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::client::auth::AuthSchemeId; use crate::client::orchestrator::Future; -use aws_smithy_http::property_bag::PropertyBag; +use crate::config_bag::ConfigBag; use std::any::Any; use std::fmt::Debug; use std::sync::Arc; @@ -14,12 +15,12 @@ use std::time::SystemTime; pub mod http; pub trait IdentityResolver: Send + Sync + Debug { - fn resolve_identity(&self, identity_properties: &PropertyBag) -> Future; + fn resolve_identity(&self, config_bag: &ConfigBag) -> Future; } #[derive(Clone, Debug, Default)] pub struct IdentityResolvers { - identity_resolvers: Vec<(&'static str, Arc)>, + identity_resolvers: Vec<(AuthSchemeId, Arc)>, } impl IdentityResolvers { @@ -27,10 +28,10 @@ impl IdentityResolvers { builders::IdentityResolversBuilder::new() } - pub fn identity_resolver(&self, identity_type: &'static str) -> Option<&dyn IdentityResolver> { + pub fn identity_resolver(&self, scheme_id: AuthSchemeId) -> Option<&dyn IdentityResolver> { self.identity_resolvers .iter() - .find(|resolver| resolver.0 == identity_type) + .find(|resolver| resolver.0 == scheme_id) .map(|resolver| &*resolver.1) } @@ -83,17 +84,18 @@ impl AnonymousIdentityResolver { } impl IdentityResolver for AnonymousIdentityResolver { - fn resolve_identity(&self, _: &PropertyBag) -> Future { + fn resolve_identity(&self, _: &ConfigBag) -> Future { Future::ready(Ok(Identity::new(AnonymousIdentity::new(), None))) } } pub mod builders { use super::*; + use crate::client::auth::AuthSchemeId; #[derive(Debug, Default)] pub struct IdentityResolversBuilder { - pub(super) identity_resolvers: Vec<(&'static str, Arc)>, + pub(super) identity_resolvers: Vec<(AuthSchemeId, Arc)>, } impl IdentityResolversBuilder { @@ -103,11 +105,11 @@ pub mod builders { pub fn identity_resolver( mut self, - name: &'static str, + scheme_id: AuthSchemeId, resolver: impl IdentityResolver + 'static, ) -> Self { self.identity_resolvers - .push((name, Arc::new(resolver) as _)); + .push((scheme_id, Arc::new(resolver) as _)); self } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs b/rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs index 5a58588b02..5866d772f4 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs @@ -7,7 +7,7 @@ use crate::client::identity::{Identity, IdentityResolver}; use crate::client::orchestrator::Future; -use aws_smithy_http::property_bag::PropertyBag; +use crate::config_bag::ConfigBag; use std::fmt::Debug; use std::sync::Arc; use std::time::SystemTime; @@ -65,7 +65,7 @@ impl From for Token { } impl IdentityResolver for Token { - fn resolve_identity(&self, _identity_properties: &PropertyBag) -> Future { + fn resolve_identity(&self, _config_bag: &ConfigBag) -> Future { Future::ready(Ok(Identity::new(self.clone(), self.0.expiration))) } } @@ -124,7 +124,7 @@ impl Login { } impl IdentityResolver for Login { - fn resolve_identity(&self, _identity_properties: &PropertyBag) -> Future { + fn resolve_identity(&self, _config_bag: &ConfigBag) -> Future { Future::ready(Ok(Identity::new(self.clone(), self.0.expiration))) } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs index 85233f12ce..db36e762d9 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -use super::identity::{IdentityResolver, IdentityResolvers}; -use crate::client::identity::Identity; +use crate::client::auth::{AuthOptionResolver, AuthOptionResolverParams, HttpAuthSchemes}; +use crate::client::identity::IdentityResolvers; use crate::client::interceptors::context::{Input, OutputOrError}; use crate::client::retries::RetryClassifiers; use crate::client::retries::RetryStrategy; @@ -13,13 +13,10 @@ use crate::type_erasure::{TypeErasedBox, TypedBox}; use aws_smithy_async::future::now_or_later::NowOrLater; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::EndpointPrefix; -use aws_smithy_http::property_bag::PropertyBag; use std::any::Any; -use std::borrow::Cow; use std::fmt::Debug; use std::future::Future as StdFuture; use std::pin::Pin; -use std::sync::Arc; use std::time::SystemTime; pub type HttpRequest = http::Request; @@ -55,104 +52,6 @@ impl Connection for Box { } } -#[derive(Debug)] -pub struct AuthOptionResolverParams(TypeErasedBox); - -impl AuthOptionResolverParams { - pub fn new(params: T) -> Self { - Self(TypedBox::new(params).erase()) - } - - pub fn get(&self) -> Option<&T> { - self.0.downcast_ref() - } -} - -pub trait AuthOptionResolver: Send + Sync + Debug { - fn resolve_auth_options<'a>( - &'a self, - params: &AuthOptionResolverParams, - ) -> Result, BoxError>; -} - -impl AuthOptionResolver for Box { - fn resolve_auth_options<'a>( - &'a self, - params: &AuthOptionResolverParams, - ) -> Result, BoxError> { - (**self).resolve_auth_options(params) - } -} - -#[derive(Clone, Debug)] -pub struct HttpAuthOption { - scheme_id: &'static str, - properties: Arc, -} - -impl HttpAuthOption { - pub fn new(scheme_id: &'static str, properties: Arc) -> Self { - Self { - scheme_id, - properties, - } - } - - pub fn scheme_id(&self) -> &'static str { - self.scheme_id - } - - pub fn properties(&self) -> &PropertyBag { - &self.properties - } -} - -#[derive(Debug)] -struct HttpAuthSchemesInner { - schemes: Vec<(&'static str, Box)>, -} -#[derive(Debug)] -pub struct HttpAuthSchemes { - inner: Arc, -} - -impl HttpAuthSchemes { - pub fn builder() -> builders::HttpAuthSchemesBuilder { - Default::default() - } - - pub fn scheme(&self, name: &'static str) -> Option<&dyn HttpAuthScheme> { - self.inner - .schemes - .iter() - .find(|scheme| scheme.0 == name) - .map(|scheme| &*scheme.1) - } -} - -pub trait HttpAuthScheme: Send + Sync + Debug { - fn scheme_id(&self) -> &'static str; - - fn identity_resolver<'a>( - &self, - identity_resolvers: &'a IdentityResolvers, - ) -> Option<&'a dyn IdentityResolver>; - - fn request_signer(&self) -> &dyn HttpRequestSigner; -} - -pub trait HttpRequestSigner: Send + Sync + Debug { - /// Return a signed version of the given request using the given identity. - /// - /// If the provided identity is incompatible with this signer, an error must be returned. - fn sign_request( - &self, - request: &mut HttpRequest, - identity: &Identity, - signing_properties: &PropertyBag, - ) -> Result<(), BoxError>; -} - #[derive(Debug)] pub struct EndpointResolverParams(TypeErasedBox); @@ -378,35 +277,3 @@ impl ConfigBagAccessors for ConfigBag { self.put::(request_time); } } - -pub mod builders { - use super::*; - - #[derive(Debug, Default)] - pub struct HttpAuthSchemesBuilder { - schemes: Vec<(&'static str, Box)>, - } - - impl HttpAuthSchemesBuilder { - pub fn new() -> Self { - Default::default() - } - - pub fn auth_scheme( - mut self, - name: &'static str, - auth_scheme: impl HttpAuthScheme + 'static, - ) -> Self { - self.schemes.push((name, Box::new(auth_scheme) as _)); - self - } - - pub fn build(self) -> HttpAuthSchemes { - HttpAuthSchemes { - inner: Arc::new(HttpAuthSchemesInner { - schemes: self.schemes, - }), - } - } - } -} diff --git a/rust-runtime/aws-smithy-runtime/src/client/auth/http.rs b/rust-runtime/aws-smithy-runtime/src/client/auth/http.rs index 8bacee21f0..a9ca17f0b3 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/auth/http.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/auth/http.rs @@ -3,17 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http::property_bag::PropertyBag; use aws_smithy_http::query_writer::QueryWriter; use aws_smithy_runtime_api::client::auth::http::{ HTTP_API_KEY_AUTH_SCHEME_ID, HTTP_BASIC_AUTH_SCHEME_ID, HTTP_BEARER_AUTH_SCHEME_ID, HTTP_DIGEST_AUTH_SCHEME_ID, }; +use aws_smithy_runtime_api::client::auth::{AuthSchemeId, HttpAuthScheme, HttpRequestSigner}; use aws_smithy_runtime_api::client::identity::http::{Login, Token}; use aws_smithy_runtime_api::client::identity::{Identity, IdentityResolver, IdentityResolvers}; -use aws_smithy_runtime_api::client::orchestrator::{ - BoxError, HttpAuthScheme, HttpRequest, HttpRequestSigner, -}; +use aws_smithy_runtime_api::client::orchestrator::{BoxError, HttpRequest}; +use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_smithy_types::base64::encode; use http::header::HeaderName; use http::HeaderValue; @@ -49,7 +48,7 @@ impl ApiKeyAuthScheme { } impl HttpAuthScheme for ApiKeyAuthScheme { - fn scheme_id(&self) -> &'static str { + fn scheme_id(&self) -> AuthSchemeId { HTTP_API_KEY_AUTH_SCHEME_ID } @@ -77,7 +76,7 @@ impl HttpRequestSigner for ApiKeySigner { &self, request: &mut HttpRequest, identity: &Identity, - _signing_properties: &PropertyBag, + _config_bag: &ConfigBag, ) -> Result<(), BoxError> { let api_key = identity .data::() @@ -118,7 +117,7 @@ impl BasicAuthScheme { } impl HttpAuthScheme for BasicAuthScheme { - fn scheme_id(&self) -> &'static str { + fn scheme_id(&self) -> AuthSchemeId { HTTP_BASIC_AUTH_SCHEME_ID } @@ -142,7 +141,7 @@ impl HttpRequestSigner for BasicAuthSigner { &self, request: &mut HttpRequest, identity: &Identity, - _signing_properties: &PropertyBag, + _config_bag: &ConfigBag, ) -> Result<(), BoxError> { let login = identity .data::() @@ -175,7 +174,7 @@ impl BearerAuthScheme { } impl HttpAuthScheme for BearerAuthScheme { - fn scheme_id(&self) -> &'static str { + fn scheme_id(&self) -> AuthSchemeId { HTTP_BEARER_AUTH_SCHEME_ID } @@ -199,7 +198,7 @@ impl HttpRequestSigner for BearerAuthSigner { &self, request: &mut HttpRequest, identity: &Identity, - _signing_properties: &PropertyBag, + _config_bag: &ConfigBag, ) -> Result<(), BoxError> { let token = identity .data::() @@ -230,7 +229,7 @@ impl DigestAuthScheme { } impl HttpAuthScheme for DigestAuthScheme { - fn scheme_id(&self) -> &'static str { + fn scheme_id(&self) -> AuthSchemeId { HTTP_DIGEST_AUTH_SCHEME_ID } @@ -254,7 +253,7 @@ impl HttpRequestSigner for DigestAuthSigner { &self, _request: &mut HttpRequest, _identity: &Identity, - _signing_properties: &PropertyBag, + _config_bag: &ConfigBag, ) -> Result<(), BoxError> { unimplemented!( "support for signing with Smithy's `@httpDigestAuth` auth scheme is not implemented yet" @@ -275,14 +274,14 @@ mod tests { location: ApiKeyLocation::Header, name: "some-header-name".into(), }; - let signing_properties = PropertyBag::new(); + let config_bag = ConfigBag::base(); let identity = Identity::new(Token::new("some-token", None), None); let mut request = http::Request::builder() .uri("http://example.com/Foobaz") .body(SdkBody::empty()) .unwrap(); signer - .sign_request(&mut request, &identity, &signing_properties) + .sign_request(&mut request, &identity, &config_bag) .expect("success"); assert_eq!( "SomeSchemeName some-token", @@ -298,14 +297,14 @@ mod tests { location: ApiKeyLocation::Query, name: "some-query-name".into(), }; - let signing_properties = PropertyBag::new(); + let config_bag = ConfigBag::base(); let identity = Identity::new(Token::new("some-token", None), None); let mut request = http::Request::builder() .uri("http://example.com/Foobaz") .body(SdkBody::empty()) .unwrap(); signer - .sign_request(&mut request, &identity, &signing_properties) + .sign_request(&mut request, &identity, &config_bag) .expect("success"); assert!(request.headers().get("some-query-name").is_none()); assert_eq!( @@ -317,12 +316,12 @@ mod tests { #[test] fn test_basic_auth() { let signer = BasicAuthSigner; - let signing_properties = PropertyBag::new(); + let config_bag = ConfigBag::base(); let identity = Identity::new(Login::new("Aladdin", "open sesame", None), None); let mut request = http::Request::builder().body(SdkBody::empty()).unwrap(); signer - .sign_request(&mut request, &identity, &signing_properties) + .sign_request(&mut request, &identity, &config_bag) .expect("success"); assert_eq!( "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==", @@ -334,11 +333,11 @@ mod tests { fn test_bearer_auth() { let signer = BearerAuthSigner; - let signing_properties = PropertyBag::new(); + let config_bag = ConfigBag::base(); let identity = Identity::new(Token::new("some-token", None), None); let mut request = http::Request::builder().body(SdkBody::empty()).unwrap(); signer - .sign_request(&mut request, &identity, &signing_properties) + .sign_request(&mut request, &identity, &config_bag) .expect("success"); assert_eq!( "Bearer some-token", diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs index 77c6b6f21d..9818593f05 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs @@ -30,20 +30,18 @@ pub(super) async fn orchestrate_auth( identity_resolvers = ?identity_resolvers, "orchestrating auth", ); - for option in auth_options.as_ref() { - let scheme_id = option.scheme_id(); - let scheme_properties = option.properties(); + for &scheme_id in auth_options.as_ref() { if let Some(auth_scheme) = cfg.http_auth_schemes().scheme(scheme_id) { if let Some(identity_resolver) = auth_scheme.identity_resolver(identity_resolvers) { let request_signer = auth_scheme.request_signer(); let identity = identity_resolver - .resolve_identity(scheme_properties) + .resolve_identity(cfg) .await .map_err(construction_failure)?; return dispatch_phase.include_mut(|ctx| { let request = ctx.request_mut()?; - request_signer.sign_request(request, &identity, scheme_properties)?; + request_signer.sign_request(request, &identity, cfg)?; Result::<_, BoxError>::Ok(()) }); } @@ -59,23 +57,21 @@ pub(super) async fn orchestrate_auth( mod tests { use super::*; use aws_smithy_http::body::SdkBody; - use aws_smithy_http::property_bag::PropertyBag; - use aws_smithy_runtime_api::client::auth::option_resolver::AuthOptionListResolver; + use aws_smithy_runtime_api::client::auth::option_resolver::StaticAuthOptionResolver; + use aws_smithy_runtime_api::client::auth::{ + AuthOptionResolverParams, AuthSchemeId, HttpAuthScheme, HttpAuthSchemes, HttpRequestSigner, + }; use aws_smithy_runtime_api::client::identity::{Identity, IdentityResolver, IdentityResolvers}; use aws_smithy_runtime_api::client::interceptors::InterceptorContext; - use aws_smithy_runtime_api::client::orchestrator::{ - AuthOptionResolverParams, Future, HttpAuthOption, HttpAuthScheme, HttpAuthSchemes, - HttpRequest, HttpRequestSigner, - }; + use aws_smithy_runtime_api::client::orchestrator::{Future, HttpRequest}; use aws_smithy_runtime_api::type_erasure::TypedBox; - use std::sync::Arc; #[tokio::test] async fn basic_case() { #[derive(Debug)] struct TestIdentityResolver; impl IdentityResolver for TestIdentityResolver { - fn resolve_identity(&self, _identity_properties: &PropertyBag) -> Future { + fn resolve_identity(&self, _config_bag: &ConfigBag) -> Future { Future::ready(Ok(Identity::new("doesntmatter", None))) } } @@ -88,7 +84,7 @@ mod tests { &self, request: &mut HttpRequest, _identity: &Identity, - _signing_properties: &PropertyBag, + _config_bag: &ConfigBag, ) -> Result<(), BoxError> { request .headers_mut() @@ -97,13 +93,15 @@ mod tests { } } + const TEST_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("test-scheme"); + #[derive(Debug)] struct TestAuthScheme { signer: TestSigner, } impl HttpAuthScheme for TestAuthScheme { - fn scheme_id(&self) -> &'static str { - "test-scheme" + fn scheme_id(&self) -> AuthSchemeId { + TEST_SCHEME_ID } fn identity_resolver<'a>( @@ -124,18 +122,15 @@ mod tests { let mut cfg = ConfigBag::base(); cfg.set_auth_option_resolver_params(AuthOptionResolverParams::new("doesntmatter")); - cfg.set_auth_option_resolver(AuthOptionListResolver::new(vec![HttpAuthOption::new( - "test-scheme", - Arc::new(PropertyBag::new()), - )])); + cfg.set_auth_option_resolver(StaticAuthOptionResolver::new(vec![TEST_SCHEME_ID])); cfg.set_identity_resolvers( IdentityResolvers::builder() - .identity_resolver("test-scheme", TestIdentityResolver) + .identity_resolver(TEST_SCHEME_ID, TestIdentityResolver) .build(), ); cfg.set_http_auth_schemes( HttpAuthSchemes::builder() - .auth_scheme("test-scheme", TestAuthScheme { signer: TestSigner }) + .auth_scheme(TEST_SCHEME_ID, TestAuthScheme { signer: TestSigner }) .build(), ); @@ -170,9 +165,9 @@ mod tests { let mut cfg = ConfigBag::base(); cfg.set_auth_option_resolver_params(AuthOptionResolverParams::new("doesntmatter")); - cfg.set_auth_option_resolver(AuthOptionListResolver::new(vec![ - HttpAuthOption::new(HTTP_BASIC_AUTH_SCHEME_ID, Arc::new(PropertyBag::new())), - HttpAuthOption::new(HTTP_BEARER_AUTH_SCHEME_ID, Arc::new(PropertyBag::new())), + cfg.set_auth_option_resolver(StaticAuthOptionResolver::new(vec![ + HTTP_BASIC_AUTH_SCHEME_ID, + HTTP_BEARER_AUTH_SCHEME_ID, ])); cfg.set_http_auth_schemes( HttpAuthSchemes::builder() From 65e376d8c657cd476f7af2dba7b6bf33c00d4766 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 27 Apr 2023 11:14:55 -0700 Subject: [PATCH 03/10] Add SRA benchmark to compare the last release against the current release (#2648) ## Motivation and Context This PR adds a benchmark to make sure we didn't regress the old middleware client implementation while making changes to it for the new orchestrator implementation. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../integration-tests/aws-sdk-s3/Cargo.toml | 14 +- .../benches/middleware_vs_orchestrator.rs | 167 +++++++++++------- 2 files changed, 108 insertions(+), 73 deletions(-) diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml b/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml index 40c9f1bdbb..27998067f1 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml +++ b/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml @@ -6,24 +6,20 @@ publish = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -aws-credential-types = { path = "../../../rust-runtime/aws-credential-types", features = ["test-util"] } aws-http = { path = "../../../rust-runtime/aws-http" } aws-runtime = { path = "../../../rust-runtime/aws-runtime" } aws-sdk-s3 = { path = "../../build/sdk/aws-sdk-s3", features = ["test-util"] } -aws-sigv4 = { path = "../../../rust-runtime/aws-sigv4" } -aws-types = { path = "../../../rust-runtime/aws-types" } -aws-smithy-async = { path = "../../../../rust-runtime/aws-smithy-async", features = ["rt-tokio"] } aws-smithy-client = { path = "../../../../rust-runtime/aws-smithy-client", features = ["test-util", "rustls"] } -aws-smithy-types = { path = "../../../../rust-runtime/aws-smithy-types" } -aws-smithy-http = { path = "../../../../rust-runtime/aws-smithy-http" } -aws-smithy-runtime = { path = "../../../../rust-runtime/aws-smithy-runtime", features = ["test-util"] } aws-smithy-runtime-api = { path = "../../../../rust-runtime/aws-smithy-runtime-api" } +aws-types = { path = "../../../rust-runtime/aws-types" } criterion = { version = "0.4", features = ["async_tokio"] } +http = "0.2.3" +http-body = "0.4.5" +last-release-smithy-client = { version = "0.55", package = "aws-smithy-client", features = ["test-util", "rustls"] } +last-release-s3 = { version = "0.26", package = "aws-sdk-s3", features = ["test-util"] } tokio = { version = "1.23.1", features = ["macros", "test-util", "rt-multi-thread"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.15", features = ["env-filter", "json"] } -http = "0.2.3" -http-body = "0.4.5" [profile.release] debug = 1 diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs index 3fba943ba1..ba9d003aa3 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -6,21 +6,84 @@ #[macro_use] extern crate criterion; use aws_sdk_s3 as s3; -use aws_smithy_client::erase::DynConnector; -use aws_smithy_client::test_connection::infallible_connection_fn; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; use aws_smithy_runtime_api::config_bag::ConfigBag; -use criterion::Criterion; -use s3::endpoint::Params; +use criterion::{BenchmarkId, Criterion}; -async fn middleware(client: &s3::Client) { - client - .list_objects_v2() - .bucket("test-bucket") - .prefix("prefix~") - .send() - .await - .expect("successful execution"); +macro_rules! test_connection { + (head) => { + test_connection!(aws_smithy_client) + }; + (last_release) => { + test_connection!(last_release_smithy_client) + }; + ($package:ident) => { + $package::test_connection::infallible_connection_fn(|req| { + assert_eq!( + "https://test-bucket.s3.us-east-1.amazonaws.com/?list-type=2&prefix=prefix~", + req.uri().to_string() + ); + assert!(req.headers().contains_key("authorization")); + http::Response::builder() + .status(200) + .body( + r#" + + test-bucket + prefix~ + 1 + 1000 + false + + some-file.file + 2009-10-12T17:50:30.000Z + 434234 + STANDARD + + +"#, + ) + .unwrap() + }) + }; +} + +macro_rules! create_client { + (head) => { + create_client!(head, aws_sdk_s3) + }; + (last_release) => { + create_client!(last_release, last_release_s3) + }; + ($original:ident, $package:ident) => {{ + let conn = test_connection!($original); + let config = $package::Config::builder() + .credentials_provider($package::config::Credentials::for_tests()) + .region($package::config::Region::new("us-east-1")) + .http_connector(conn.clone()) + .build(); + $package::Client::from_conf(config) + }}; +} + +macro_rules! middleware_bench_fn { + ($fn_name:ident, head) => { + middleware_bench_fn!($fn_name, aws_sdk_s3) + }; + ($fn_name:ident, last_release) => { + middleware_bench_fn!($fn_name, last_release_s3) + }; + ($fn_name:ident, $package:ident) => { + async fn $fn_name(client: &$package::Client) { + client + .list_objects_v2() + .bucket("test-bucket") + .prefix("prefix~") + .send() + .await + .expect("successful execution"); + } + }; } async fn orchestrator(client: &s3::Client) { @@ -32,7 +95,7 @@ async fn orchestrator(client: &s3::Client) { &self, cfg: &mut ConfigBag, ) -> Result<(), aws_smithy_runtime_api::client::runtime_plugin::BoxError> { - let params_builder = Params::builder() + let params_builder = s3::endpoint::Params::builder() .set_region(Some(self.region.clone())) .bucket("test-bucket"); @@ -55,61 +118,37 @@ async fn orchestrator(client: &s3::Client) { .expect("successful execution"); } -fn test_connection() -> DynConnector { - infallible_connection_fn(|req| { - assert_eq!( - "https://test-bucket.s3.us-east-1.amazonaws.com/?list-type=2&prefix=prefix~", - req.uri().to_string() - ); - assert!(req.headers().contains_key("authorization")); - http::Response::builder() - .status(200) - .body( - r#" - - test-bucket - prefix~ - 1 - 1000 - false - - some-file.file - 2009-10-12T17:50:30.000Z - 434234 - STANDARD - - -"#, - ) - .unwrap() - }) -} - -fn client() -> s3::Client { - let conn = test_connection(); - let config = s3::Config::builder() - .credentials_provider(s3::config::Credentials::for_tests()) - .region(s3::config::Region::new("us-east-1")) - .http_connector(conn.clone()) - .build(); - s3::Client::from_conf(config) -} +fn bench(c: &mut Criterion) { + let head_client = create_client!(head); + middleware_bench_fn!(middleware_head, head); -fn middleware_bench(c: &mut Criterion) { - let client = client(); - c.bench_function("middleware", move |b| { - b.to_async(tokio::runtime::Runtime::new().unwrap()) - .iter(|| async { middleware(&client).await }) - }); -} + let last_release_client = create_client!(last_release); + middleware_bench_fn!(middleware_last_release, last_release); -fn orchestrator_bench(c: &mut Criterion) { - let client = client(); - c.bench_function("orchestrator", move |b| { + let mut group = c.benchmark_group("compare"); + let param = "S3 ListObjectsV2"; + group.bench_with_input( + BenchmarkId::new("middleware (HEAD)", param), + param, + |b, _| { + b.to_async(tokio::runtime::Runtime::new().unwrap()) + .iter(|| async { middleware_head(&head_client).await }) + }, + ); + group.bench_with_input( + BenchmarkId::new("middleware (last_release)", param), + param, + |b, _| { + b.to_async(tokio::runtime::Runtime::new().unwrap()) + .iter(|| async { middleware_last_release(&last_release_client).await }) + }, + ); + group.bench_with_input(BenchmarkId::new("orchestrator", param), param, |b, _| { b.to_async(tokio::runtime::Runtime::new().unwrap()) - .iter(|| async { orchestrator(&client).await }) + .iter(|| async { orchestrator(&head_client).await }) }); + group.finish(); } -criterion_group!(benches, middleware_bench, orchestrator_bench); +criterion_group!(benches, bench); criterion_main!(benches); From 12338ad54e99de525e2d6b979f79716d23c7100f Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 28 Apr 2023 14:31:04 -0700 Subject: [PATCH 04/10] Change the orchestrator feature gate into a quad state (#2657) ## Motivation and Context So far we have been testing with the orchestrator on or off, with "on" meaning that both the middleware and orchestrator implementations exist, and you have to opt into the orchestrator via a `send_v2` function call (instead of `send`). This approach makes it difficult to test against the orchestrator exclusively, and also test the existing integration tests with the orchestrator implementation. This PR changes `enableNewSmithyRuntime` into a quad-state with: - middleware - both middleware and orchestrator, defaulting to middleware - both middleware and orchestrator, defaulting to orchestrator - orchestrator This allows for generating a client with the orchestrator exclusively, or generating both and defaulting to the orchestrator, to reduce testing friction. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- aws/sdk-adhoc-test/build.gradle.kts | 2 - .../smithy/rustsdk/CredentialProviders.kt | 2 +- .../HttpConnectorConfigCustomization.kt | 2 +- .../rustsdk/HttpRequestChecksumDecorator.kt | 2 +- .../rustsdk/HttpResponseChecksumDecorator.kt | 2 +- .../rustsdk/IntegrationTestDependencies.kt | 2 +- .../smithy/rustsdk/InvocationIdDecorator.kt | 2 +- .../rustsdk/RecursionDetectionDecorator.kt | 2 +- .../smithy/rustsdk/SigV4AuthDecorator.kt | 4 +- .../smithy/rustsdk/UserAgentDecorator.kt | 2 +- aws/sdk/build.gradle.kts | 3 +- aws/sra-test/build.gradle.kts | 3 +- .../benches/middleware_vs_orchestrator.rs | 2 +- .../aws-sdk-s3/tests/sra_test.rs | 2 +- .../client/smithy/ClientCodegenContext.kt | 4 +- .../client/smithy/ClientRustSettings.kt | 45 +++++- .../customizations/ApiKeyAuthDecorator.kt | 2 +- .../customizations/EndpointPrefixGenerator.kt | 2 +- .../customizations/HttpAuthDecorator.kt | 10 +- .../HttpConnectorConfigDecorator.kt | 2 +- .../endpoint/EndpointParamsDecorator.kt | 2 +- .../EndpointParamsInterceptorGenerator.kt | 2 +- .../EndpointTraitBindingGenerator.kt | 5 +- .../smithy/generators/ServiceGenerator.kt | 2 +- .../client/FluentClientDecorator.kt | 2 +- .../client/FluentClientGenerator.kt | 147 +++++++++++------- .../protocol/ClientProtocolGenerator.kt | 2 +- .../customizations/HttpAuthDecoratorTest.kt | 16 +- .../generators/EndpointTraitBindingsTest.kt | 8 +- 29 files changed, 180 insertions(+), 103 deletions(-) diff --git a/aws/sdk-adhoc-test/build.gradle.kts b/aws/sdk-adhoc-test/build.gradle.kts index 98ad953105..fbd40ae1ca 100644 --- a/aws/sdk-adhoc-test/build.gradle.kts +++ b/aws/sdk-adhoc-test/build.gradle.kts @@ -46,7 +46,6 @@ val allCodegenTests = listOf( , "codegen": { "includeFluentClient": false, - "enableNewCrateOrganizationScheme": true }, "customizationConfig": { "awsSdk": { @@ -63,7 +62,6 @@ val allCodegenTests = listOf( , "codegen": { "includeFluentClient": false, - "enableNewCrateOrganizationScheme": true }, "customizationConfig": { "awsSdk": { 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 84d644a43c..eb03212135 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 @@ -32,7 +32,7 @@ class CredentialsProviderDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(CredentialsIdentityResolverRegistration(codegenContext)) } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpConnectorConfigCustomization.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpConnectorConfigCustomization.kt index 75c252d0fb..d64ddea9b3 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpConnectorConfigCustomization.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpConnectorConfigCustomization.kt @@ -26,7 +26,7 @@ class HttpConnectorDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(!codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.exclusivelyGenerateMiddleware) { it + HttpConnectorConfigCustomization(codegenContext) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpRequestChecksumDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpRequestChecksumDecorator.kt index f2913a1aa7..b72293b1fa 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpRequestChecksumDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpRequestChecksumDecorator.kt @@ -44,7 +44,7 @@ class HttpRequestChecksumDecorator : ClientCodegenDecorator { // TODO(enableNewSmithyRuntime): Implement checksumming via interceptor and delete this decorator private fun applies(codegenContext: ClientCodegenContext): Boolean = - !codegenContext.settings.codegenConfig.enableNewSmithyRuntime + codegenContext.smithyRuntimeMode.exclusivelyGenerateMiddleware override fun operationCustomizations( codegenContext: ClientCodegenContext, diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpResponseChecksumDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpResponseChecksumDecorator.kt index 832ab7e07d..cebc4e5cba 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpResponseChecksumDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/HttpResponseChecksumDecorator.kt @@ -35,7 +35,7 @@ class HttpResponseChecksumDecorator : ClientCodegenDecorator { // TODO(enableNewSmithyRuntime): Implement checksumming via interceptor and delete this decorator private fun applies(codegenContext: ClientCodegenContext, operationShape: OperationShape): Boolean = - !codegenContext.settings.codegenConfig.enableNewSmithyRuntime && operationShape.outputShape != ShapeId.from("com.amazonaws.s3#GetObjectOutput") + codegenContext.smithyRuntimeMode.generateMiddleware && operationShape.outputShape != ShapeId.from("com.amazonaws.s3#GetObjectOutput") override fun operationCustomizations( codegenContext: ClientCodegenContext, diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt index 6eedc44a89..556e71d2b0 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt @@ -126,7 +126,7 @@ class S3TestDependencies(private val codegenContext: ClientCodegenContext) : Lib // TODO(enableNewSmithyRuntime): These additional dependencies may not be needed anymore when removing this flag // depending on if the sra-test is kept around or not. - if (codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + if (codegenContext.smithyRuntimeMode.generateOrchestrator) { addDependency(CargoDependency.smithyRuntime(codegenContext.runtimeConfig).toDevDependency()) addDependency(CargoDependency.smithyRuntimeApi(codegenContext.runtimeConfig).toDevDependency()) } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt index abf317570f..c608f1c573 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt @@ -21,7 +21,7 @@ class InvocationIdDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(InvocationIdRuntimePluginCustomization(codegenContext)) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt index 09e6a30742..4685acad75 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RecursionDetectionDecorator.kt @@ -22,7 +22,7 @@ class RecursionDetectionDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(RecursionDetectionRuntimePluginCustomization(codegenContext)) } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt index f4973330c5..6131e50c79 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4AuthDecorator.kt @@ -33,7 +33,7 @@ class SigV4AuthDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(AuthServiceRuntimePluginCustomization(codegenContext)) } @@ -42,7 +42,7 @@ class SigV4AuthDecorator : ClientCodegenDecorator { operation: OperationShape, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(AuthOperationRuntimePluginCustomization(codegenContext)) } } 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 0e83d2c99d..90183fbf93 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 @@ -55,7 +55,7 @@ class UserAgentDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(AddApiMetadataIntoConfigBag(codegenContext)) } diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 06355aaab6..1f92a77b75 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -102,8 +102,7 @@ fun generateSmithyBuild(services: AwsServices): String { "renameErrors": false, "debugMode": $debugMode, "eventStreamAllowList": [$eventStreamAllowListMembers], - "enableNewCrateOrganizationScheme": true, - "enableNewSmithyRuntime": false + "enableNewSmithyRuntime": "middleware" }, "service": "${service.service}", "module": "$moduleName", diff --git a/aws/sra-test/build.gradle.kts b/aws/sra-test/build.gradle.kts index 7344916af0..669f231741 100644 --- a/aws/sra-test/build.gradle.kts +++ b/aws/sra-test/build.gradle.kts @@ -65,7 +65,8 @@ val allCodegenTests = servicesToGenerate.map { , "codegen": { "includeFluentClient": false, - "enableNewSmithyRuntime": true + ${ ""/* "enableNewSmithyRuntime": "both_default_middleware" */ } + "enableNewSmithyRuntime": "orchestrator" }, "customizationConfig": { "awsSdk": { diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs index ba9d003aa3..622cdaca44 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -107,7 +107,7 @@ async fn orchestrator(client: &s3::Client) { .list_objects_v2() .bucket("test-bucket") .prefix("prefix~") - .send_v2_with_plugin(Some(FixupPlugin { + .send_orchestrator_with_plugin(Some(FixupPlugin { region: client .conf() .region() diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index 464c815d68..3fe7e6373e 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -41,7 +41,7 @@ async fn sra_test() { .config_override(aws_sdk_s3::Config::builder().force_path_style(false)) .bucket("test-bucket") .prefix("prefix~") - .send_v2_with_plugin(Some(fixup)) + .send_orchestrator_with_plugin(Some(fixup)) .await ); // To regenerate the test: diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientCodegenContext.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientCodegenContext.kt index e0ec0afb6e..92a8a9f8ca 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientCodegenContext.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientCodegenContext.kt @@ -32,4 +32,6 @@ data class ClientCodegenContext( val rootDecorator: ClientCodegenDecorator, ) : CodegenContext( model, symbolProvider, moduleDocProvider, serviceShape, protocol, settings, CodegenTarget.CLIENT, -) +) { + val smithyRuntimeMode: SmithyRuntimeMode get() = settings.codegenConfig.enableNewSmithyRuntime +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustSettings.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustSettings.kt index e6680d3e0e..d7246cf986 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustSettings.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustSettings.kt @@ -72,6 +72,43 @@ data class ClientRustSettings( } } +// TODO(enableNewSmithyRuntime): Remove this mode after switching to the orchestrator +enum class SmithyRuntimeMode { + Middleware, + BothDefaultMiddleware, + BothDefaultOrchestrator, + Orchestrator, + ; + + val exclusivelyGenerateMiddleware: Boolean get() = generateMiddleware && !generateOrchestrator + + val generateMiddleware: Boolean get() = when (this) { + Middleware, BothDefaultMiddleware, BothDefaultOrchestrator -> true + else -> false + } + + val generateOrchestrator: Boolean get() = when (this) { + Orchestrator, BothDefaultMiddleware, BothDefaultOrchestrator -> true + else -> false + } + + val defaultToMiddleware: Boolean get() = when (this) { + Middleware, BothDefaultMiddleware -> true + else -> false + } + val defaultToOrchestrator: Boolean get() = !defaultToMiddleware + + companion object { + fun fromString(value: String): SmithyRuntimeMode = when (value) { + "middleware" -> Middleware + "orchestrator" -> Orchestrator + "both_default_middleware" -> BothDefaultMiddleware + "both_default_orchestrator" -> BothDefaultOrchestrator + else -> throw IllegalArgumentException("unknown runtime mode: $value") + } + } +} + /** * [renameExceptions]: Rename `Exception` to `Error` in the generated SDK * [includeFluentClient]: Generate a `client` module in the generated SDK (currently the AWS SDK sets this to `false` @@ -87,7 +124,7 @@ data class ClientCodegenConfig( // TODO(EventStream): [CLEANUP] Remove this property when turning on Event Stream for all services val eventStreamAllowList: Set = defaultEventStreamAllowList, // TODO(SmithyRuntime): Remove this once we commit to switch to aws-smithy-runtime and aws-smithy-runtime-api - val enableNewSmithyRuntime: Boolean = defaultEnableNewSmithyRuntime, + val enableNewSmithyRuntime: SmithyRuntimeMode = defaultEnableNewSmithyRuntime, ) : CoreCodegenConfig( formatTimeoutSeconds, debugMode, ) { @@ -96,7 +133,7 @@ data class ClientCodegenConfig( private const val defaultIncludeFluentClient = true private const val defaultAddMessageToErrors = true private val defaultEventStreamAllowList: Set = emptySet() - private const val defaultEnableNewSmithyRuntime = false + private val defaultEnableNewSmithyRuntime = SmithyRuntimeMode.Middleware fun fromCodegenConfigAndNode(coreCodegenConfig: CoreCodegenConfig, node: Optional) = if (node.isPresent) { @@ -109,7 +146,9 @@ data class ClientCodegenConfig( renameExceptions = node.get().getBooleanMemberOrDefault("renameErrors", defaultRenameExceptions), includeFluentClient = node.get().getBooleanMemberOrDefault("includeFluentClient", defaultIncludeFluentClient), addMessageToErrors = node.get().getBooleanMemberOrDefault("addMessageToErrors", defaultAddMessageToErrors), - enableNewSmithyRuntime = node.get().getBooleanMemberOrDefault("enableNewSmithyRuntime", defaultEnableNewSmithyRuntime), + enableNewSmithyRuntime = SmithyRuntimeMode.fromString( + node.get().getStringMemberOrDefault("enableNewSmithyRuntime", "middleware"), + ), ) } else { ClientCodegenConfig( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ApiKeyAuthDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ApiKeyAuthDecorator.kt index 58fd14e6a5..14fe1fd1c7 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ApiKeyAuthDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ApiKeyAuthDecorator.kt @@ -38,7 +38,7 @@ class ApiKeyAuthDecorator : ClientCodegenDecorator { override val order: Byte = 10 private fun applies(codegenContext: ClientCodegenContext) = - !codegenContext.settings.codegenConfig.enableNewSmithyRuntime && + codegenContext.smithyRuntimeMode.generateMiddleware && isSupportedApiKeyAuth(codegenContext) override fun configCustomizations( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/EndpointPrefixGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/EndpointPrefixGenerator.kt index 2fe90239de..190aac3680 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/EndpointPrefixGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/EndpointPrefixGenerator.kt @@ -32,7 +32,7 @@ class EndpointPrefixGenerator(private val codegenContext: ClientCodegenContext, endpointTraitBindings.render( this, "self", - codegenContext.settings.codegenConfig.enableNewSmithyRuntime, + codegenContext.smithyRuntimeMode, ) } rust("request.properties_mut().insert(endpoint_prefix);") diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt index 53aa5ee2aa..5b0eba15da 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecorator.kt @@ -68,12 +68,12 @@ private data class HttpAuthSchemes( companion object { fun from(codegenContext: ClientCodegenContext): HttpAuthSchemes { val authSchemes = ServiceIndex.of(codegenContext.model).getAuthSchemes(codegenContext.serviceShape).keys - val newRuntimeEnabled = codegenContext.settings.codegenConfig.enableNewSmithyRuntime + val generateOrchestrator = codegenContext.smithyRuntimeMode.generateOrchestrator return HttpAuthSchemes( - apiKey = newRuntimeEnabled && authSchemes.contains(HttpApiKeyAuthTrait.ID), - basic = newRuntimeEnabled && authSchemes.contains(HttpBasicAuthTrait.ID), - bearer = newRuntimeEnabled && authSchemes.contains(HttpBearerAuthTrait.ID), - digest = newRuntimeEnabled && authSchemes.contains(HttpDigestAuthTrait.ID), + apiKey = generateOrchestrator && authSchemes.contains(HttpApiKeyAuthTrait.ID), + basic = generateOrchestrator && authSchemes.contains(HttpBasicAuthTrait.ID), + bearer = generateOrchestrator && authSchemes.contains(HttpBearerAuthTrait.ID), + digest = generateOrchestrator && authSchemes.contains(HttpDigestAuthTrait.ID), ) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt index 102c50e718..d5ecd078b1 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt @@ -25,7 +25,7 @@ class HttpConnectorConfigDecorator : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + HttpConnectorConfigCustomization(codegenContext) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt index ba728bcdf8..f35becb55a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt @@ -31,7 +31,7 @@ class EndpointParamsDecorator : ClientCodegenDecorator { operation: OperationShape, baseCustomizations: List, ): List = - baseCustomizations.letIf(codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + baseCustomizations.letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { it + listOf(EndpointParametersRuntimePluginCustomization(codegenContext, operation)) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt index b00a020508..2dc2f2e1e8 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt @@ -118,7 +118,7 @@ class EndpointParamsInterceptorGenerator( endpointTraitBindings.render( this, "_input", - codegenContext.settings.codegenConfig.enableNewSmithyRuntime, + codegenContext.smithyRuntimeMode, ) } rust("cfg.put(endpoint_prefix);") diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt index 4cc4b93347..f02cb5cde2 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingGenerator.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.traits.EndpointTrait +import software.amazon.smithy.rust.codegen.client.smithy.SmithyRuntimeMode import software.amazon.smithy.rust.codegen.client.smithy.generators.http.rustFormatString import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.rust @@ -44,7 +45,7 @@ class EndpointTraitBindings( * * The returned expression is a `Result` */ - fun render(writer: RustWriter, input: String, enableNewSmithyRuntime: Boolean) { + fun render(writer: RustWriter, input: String, smithyRuntimeMode: SmithyRuntimeMode) { // the Rust format pattern to make the endpoint prefix e.g. "{}.foo" val formatLiteral = endpointTrait.prefixFormatString() if (endpointTrait.hostPrefix.labels.isEmpty()) { @@ -67,7 +68,7 @@ class EndpointTraitBindings( // NOTE: this is dead code until we start respecting @required rust("let $field = &$input.$field;") } - val contents = if (enableNewSmithyRuntime) { + val contents = if (smithyRuntimeMode.generateOrchestrator) { // TODO(enableNewSmithyRuntime): Remove the allow attribute once all places need .into method """ if $field.is_empty() { diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt index 764efb35df..050e124376 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt @@ -47,7 +47,7 @@ class ServiceGenerator( ) serviceConfigGenerator.render(this) - if (codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + if (codegenContext.smithyRuntimeMode.generateOrchestrator) { // Enable users to opt in to the test-utils in the runtime crate rustCrate.mergeFeature(TestUtilFeature.copy(deps = listOf("aws-smithy-runtime/test-util"))) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt index e5e2760cee..7fb225fa48 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDecorator.kt @@ -36,7 +36,7 @@ class FluentClientDecorator : ClientCodegenDecorator { return } - val generics = if (codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + val generics = if (codegenContext.smithyRuntimeMode.generateOrchestrator) { NoClientGenerics(codegenContext.runtimeConfig) } else { FlexibleClientGenerics( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index 6c085fd673..f10de15ee4 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -78,7 +78,7 @@ class FluentClientGenerator( private val model = codegenContext.model private val runtimeConfig = codegenContext.runtimeConfig private val core = FluentClientCore(model) - private val enableNewSmithyRuntime = codegenContext.settings.codegenConfig.enableNewSmithyRuntime + private val smithyRuntimeMode = codegenContext.smithyRuntimeMode fun render(crate: RustCrate) { renderFluentClient(crate) @@ -255,7 +255,7 @@ class FluentClientGenerator( "Inner" to symbolProvider.symbolForBuilder(input), "generics" to generics.decl, ) - if (enableNewSmithyRuntime) { + if (smithyRuntimeMode.generateOrchestrator) { rust("config_override: std::option::Option,") } } @@ -279,11 +279,26 @@ class FluentClientGenerator( "}", ) { rust("handle, inner: Default::default(),") - if (enableNewSmithyRuntime) { + if (smithyRuntimeMode.generateOrchestrator) { rust("config_override: None,") } } } + val middlewareScope = arrayOf( + "CustomizableOperation" to ClientRustModule.Client.customize.toType() + .resolve("CustomizableOperation"), + "ClassifyRetry" to RuntimeType.classifyRetry(runtimeConfig), + "OperationError" to errorType, + "OperationOutput" to outputType, + "SdkError" to RuntimeType.sdkError(runtimeConfig), + "SdkSuccess" to RuntimeType.sdkSuccess(runtimeConfig), + "send_bounds" to generics.sendBounds(operationSymbol, outputType, errorType, retryClassifier), + "customizable_op_type_params" to rustTypeParameters( + symbolProvider.toSymbol(operation), + retryClassifier, + generics.toRustGenerics(), + ), + ) rustTemplate( """ /// Consume this builder, creating a customizable operation that can be modified before being @@ -300,15 +315,9 @@ class FluentClientGenerator( Ok(#{CustomizableOperation} { handle, operation }) } - /// Sends the request and returns the response. - /// - /// If an error occurs, an `SdkError` will be returned with additional details that - /// can be matched against. - /// - /// By default, any retryable failures will be retried twice. Retry behavior - /// is configurable with the [RetryConfig](aws_smithy_types::retry::RetryConfig), which can be - /// set when configuring the client. - pub async fn send(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}>> + // This function will go away in the near future. Do not rely on it. + ##[doc(hidden)] + pub async fn send_middleware(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}>> #{send_bounds:W} { let op = self.inner.build().map_err(#{SdkError}::construction_failure)? .make_operation(&self.handle.conf) @@ -317,24 +326,11 @@ class FluentClientGenerator( self.handle.client.call(op).await } """, - "CustomizableOperation" to ClientRustModule.Client.customize.toType() - .resolve("CustomizableOperation"), - "ClassifyRetry" to RuntimeType.classifyRetry(runtimeConfig), - "OperationError" to errorType, - "OperationOutput" to outputType, - "SdkError" to RuntimeType.sdkError(runtimeConfig), - "SdkSuccess" to RuntimeType.sdkSuccess(runtimeConfig), - "send_bounds" to generics.sendBounds(operationSymbol, outputType, errorType, retryClassifier), - "customizable_op_type_params" to rustTypeParameters( - symbolProvider.toSymbol(operation), - retryClassifier, - generics.toRustGenerics(), - ), + *middlewareScope, ) - if (enableNewSmithyRuntime) { + if (smithyRuntimeMode.defaultToMiddleware) { rustTemplate( """ - // TODO(enableNewSmithyRuntime): Replace `send` with `send_v2` /// Sends the request and returns the response. /// /// If an error occurs, an `SdkError` will be returned with additional details that @@ -343,13 +339,40 @@ class FluentClientGenerator( /// By default, any retryable failures will be retried twice. Retry behavior /// is configurable with the [RetryConfig](aws_smithy_types::retry::RetryConfig), which can be /// set when configuring the client. - pub async fn send_v2(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> { - self.send_v2_with_plugin(Option::>::None).await + pub async fn send(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}>> + #{send_bounds:W} { + self.send_middleware().await } + """, + *middlewareScope, + ) + } + if (smithyRuntimeMode.generateOrchestrator) { + val orchestratorScope = arrayOf( + "HttpResponse" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::orchestrator::HttpResponse"), + "OperationError" to errorType, + "Operation" to symbolProvider.toSymbol(operation), + "OperationOutput" to outputType, + "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), + "RuntimePlugins" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::runtime_plugin::RuntimePlugins"), + "SdkError" to RuntimeType.sdkError(runtimeConfig), + "TypedBox" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("type_erasure::TypedBox"), + "invoke" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::orchestrator::invoke"), + ) + rustTemplate( + """ + ##[doc(hidden)] + pub async fn send_orchestrator(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> { + self.send_orchestrator_with_plugin(Option::>::None).await + } + + ##[doc(hidden)] // TODO(enableNewSmithyRuntime): Delete when unused - /// Equivalent to [`Self::send_v2`] but adds a final runtime plugin to shim missing behavior - pub async fn send_v2_with_plugin(self, final_plugin: Option) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> { + /// Equivalent to [`Self::send_orchestrator`] but adds a final runtime plugin to shim missing behavior + pub async fn send_orchestrator_with_plugin(self, final_plugin: Option) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> { let mut runtime_plugins = #{RuntimePlugins}::new() .with_client_plugin(crate::config::ServiceRuntimePlugin::new(self.handle.clone())); if let Some(config_override) = self.config_override { @@ -373,18 +396,26 @@ class FluentClientGenerator( Ok(#{TypedBox}::<#{OperationOutput}>::assume_from(output).expect("correct output type").unwrap()) } """, - "HttpResponse" to RuntimeType.smithyRuntimeApi(runtimeConfig) - .resolve("client::orchestrator::HttpResponse"), - "OperationError" to errorType, - "Operation" to symbolProvider.toSymbol(operation), - "OperationOutput" to outputType, - "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), - "RuntimePlugins" to RuntimeType.smithyRuntimeApi(runtimeConfig) - .resolve("client::runtime_plugin::RuntimePlugins"), - "SdkError" to RuntimeType.sdkError(runtimeConfig), - "TypedBox" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("type_erasure::TypedBox"), - "invoke" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::orchestrator::invoke"), + *orchestratorScope, ) + if (smithyRuntimeMode.defaultToOrchestrator) { + rustTemplate( + """ + /// Sends the request and returns the response. + /// + /// If an error occurs, an `SdkError` will be returned with additional details that + /// can be matched against. + /// + /// By default, any retryable failures will be retried twice. Retry behavior + /// is configurable with the [RetryConfig](aws_smithy_types::retry::RetryConfig), which can be + /// set when configuring the client. + pub async fn send(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> { + self.send_orchestrator().await + } + """, + *orchestratorScope, + ) + } rustTemplate( """ @@ -426,20 +457,24 @@ class FluentClientGenerator( """, ) } - PaginatorGenerator.paginatorType(codegenContext, generics, operation, retryClassifier) - ?.also { paginatorType -> - rustTemplate( - """ - /// Create a paginator for this request - /// - /// Paginators are used by calling [`send().await`](#{Paginator}::send) which returns a `Stream`. - pub fn into_paginator(self) -> #{Paginator}${generics.inst} { - #{Paginator}::new(self.handle, self.inner) - } - """, - "Paginator" to paginatorType, - ) - } + + // TODO(enableNewSmithyRuntime): Port paginators to the orchestrator + if (smithyRuntimeMode.generateMiddleware) { + PaginatorGenerator.paginatorType(codegenContext, generics, operation, retryClassifier) + ?.also { paginatorType -> + rustTemplate( + """ + /// Create a paginator for this request + /// + /// Paginators are used by calling [`send().await`](#{Paginator}::send) which returns a `Stream`. + pub fn into_paginator(self) -> #{Paginator}${generics.inst} { + #{Paginator}::new(self.handle, self.inner) + } + """, + "Paginator" to paginatorType, + ) + } + } writeCustomizations( customizations, FluentClientSection.FluentBuilderImpl( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ClientProtocolGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ClientProtocolGenerator.kt index 3c8fc7c974..85dcb206db 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ClientProtocolGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ClientProtocolGenerator.kt @@ -97,7 +97,7 @@ open class ClientProtocolGenerator( } traitGenerator.generateTraitImpls(operationWriter, operationShape, operationCustomizations) - if (codegenContext.settings.codegenConfig.enableNewSmithyRuntime) { + if (codegenContext.smithyRuntimeMode.generateOrchestrator) { OperationRuntimePluginGenerator(codegenContext).render( operationWriter, operationShape, diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt index 29c3b2e853..bcb5ec2f4e 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt @@ -6,8 +6,8 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations import org.junit.jupiter.api.Test -import software.amazon.smithy.model.node.BooleanNode import software.amazon.smithy.model.node.ObjectNode +import software.amazon.smithy.model.node.StringNode import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency @@ -22,7 +22,7 @@ private fun additionalSettings(): ObjectNode = ObjectNode.objectNodeBuilder() .withMember( "codegen", ObjectNode.objectNodeBuilder() - .withMember("enableNewSmithyRuntime", BooleanNode.from(true)).build(), + .withMember("enableNewSmithyRuntime", StringNode.from("orchestrator")).build(), ) .build() @@ -67,7 +67,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); @@ -101,7 +101,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); @@ -146,7 +146,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); @@ -192,7 +192,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); @@ -238,7 +238,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); @@ -284,7 +284,7 @@ class HttpAuthDecoratorTest { .build_dyn(); let client = $moduleName::Client::with_config(smithy_client, config); let _ = client.some_operation() - .send_v2() + .send_orchestrator() .await .expect("success"); connector.assert_requests_match(&[]); diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt index 6fe1328bb8..34225e8dfd 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/EndpointTraitBindingsTest.kt @@ -11,6 +11,7 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.traits.EndpointTrait +import software.amazon.smithy.rust.codegen.client.smithy.SmithyRuntimeMode import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest import software.amazon.smithy.rust.codegen.client.testutil.testSymbolProvider import software.amazon.smithy.rust.codegen.core.rustlang.Attribute @@ -37,8 +38,9 @@ internal class EndpointTraitBindingsTest { } @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `generate endpoint prefixes`(enableNewSmithyRuntime: Boolean) { + @ValueSource(strings = ["middleware", "orchestrator"]) + fun `generate endpoint prefixes`(smithyRuntimeModeStr: String) { + val smithyRuntimeMode = SmithyRuntimeMode.fromString(smithyRuntimeModeStr) val model = """ namespace test @readonly @@ -76,7 +78,7 @@ internal class EndpointTraitBindingsTest { RuntimeType.smithyHttp(TestRuntimeConfig), TestRuntimeConfig.operationBuildError(), ) { - endpointBindingGenerator.render(this, "self", enableNewSmithyRuntime) + endpointBindingGenerator.render(this, "self", smithyRuntimeMode) } } unitTest( From 9361aa52345c23ae4943cb8f0e4155b801414a58 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 28 Apr 2023 14:59:30 -0700 Subject: [PATCH 05/10] Generate endpoint params in the orchestrator codegen (#2658) ## Motivation and Context This PR adds the codegen logic to generate endpoint parameters in the endpoint params interceptor. Fixes #2644. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../aws-sdk-s3/tests/sra_test.rs | 8 -- .../endpoint/EndpointParamsDecorator.kt | 4 - .../smithy/endpoint/EndpointsDecorator.kt | 1 + .../EndpointParamsInterceptorGenerator.kt | 135 ++++++++++-------- .../ServiceRuntimePluginGenerator.kt | 8 +- 5 files changed, 83 insertions(+), 73 deletions(-) diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index 3fe7e6373e..bb7c5d4d31 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -6,7 +6,6 @@ use aws_http::user_agent::AwsUserAgent; use aws_runtime::invocation_id::InvocationId; use aws_sdk_s3::config::{Credentials, Region}; -use aws_sdk_s3::endpoint::Params; use aws_sdk_s3::Client; use aws_smithy_client::dvr; use aws_smithy_client::dvr::MediaType; @@ -31,7 +30,6 @@ async fn sra_test() { .build(); let client = Client::from_conf(config); let fixup = FixupPlugin { - client: client.clone(), timestamp: UNIX_EPOCH + Duration::from_secs(1624036048), }; @@ -52,7 +50,6 @@ async fn sra_test() { } struct FixupPlugin { - client: Client, timestamp: SystemTime, } impl RuntimePlugin for FixupPlugin { @@ -60,11 +57,6 @@ impl RuntimePlugin for FixupPlugin { &self, cfg: &mut ConfigBag, ) -> Result<(), aws_smithy_runtime_api::client::runtime_plugin::BoxError> { - let params_builder = Params::builder() - .set_region(self.client.conf().region().map(|c| c.as_ref().to_string())) - .bucket("test-bucket"); - - cfg.put(params_builder); cfg.set_request_time(RequestTime::new(self.timestamp.clone())); cfg.put(AwsUserAgent::for_tests()); cfg.put(InvocationId::for_tests()); diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt index f35becb55a..494b6412f8 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsDecorator.kt @@ -47,10 +47,6 @@ private class EndpointParametersRuntimePluginCustomization( section.registerInterceptor(codegenContext.runtimeConfig, this) { rust("${operationName}EndpointParamsInterceptor") } - // The finalizer interceptor should be registered last - section.registerInterceptor(codegenContext.runtimeConfig, this) { - rust("${operationName}EndpointParamsFinalizerInterceptor") - } } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt index 8aa3990850..bc101412af 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt @@ -101,6 +101,7 @@ class EndpointsDecorator : ClientCodegenDecorator { override val name: String = "Endpoints" override val order: Byte = 0 + // TODO(enableNewSmithyRuntime): Remove `operationCustomizations` and `InjectEndpointInMakeOperation` override fun operationCustomizations( codegenContext: ClientCodegenContext, operation: OperationShape, diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt index 2dc2f2e1e8..d79ad7b600 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt @@ -5,10 +5,17 @@ package software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators +import software.amazon.smithy.model.node.BooleanNode +import software.amazon.smithy.model.node.Node +import software.amazon.smithy.model.node.StringNode import software.amazon.smithy.model.shapes.OperationShape +import software.amazon.smithy.model.shapes.ShapeType import software.amazon.smithy.model.traits.EndpointTrait +import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameters +import software.amazon.smithy.rulesengine.traits.ContextIndex import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointTypesGenerator +import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName import software.amazon.smithy.rust.codegen.client.smithy.generators.EndpointTraitBindings import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter @@ -17,13 +24,17 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable +import software.amazon.smithy.rust.codegen.core.util.PANIC +import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.inputShape +import software.amazon.smithy.rust.codegen.core.util.orNull class EndpointParamsInterceptorGenerator( private val codegenContext: ClientCodegenContext, ) { private val model = codegenContext.model private val symbolProvider = codegenContext.symbolProvider + private val endpointTypesGenerator = EndpointTypesGenerator.fromContext(codegenContext) private val codegenScope = codegenContext.runtimeConfig.let { rc -> val endpointTypesGenerator = EndpointTypesGenerator.fromContext(codegenContext) val runtimeApi = CargoDependency.smithyRuntimeApi(rc).toType() @@ -34,29 +45,19 @@ class EndpointParamsInterceptorGenerator( "ConfigBag" to runtimeApi.resolve("config_bag::ConfigBag"), "ContextAttachedError" to interceptors.resolve("error::ContextAttachedError"), "EndpointResolverParams" to orchestrator.resolve("EndpointResolverParams"), - "HttpResponse" to orchestrator.resolve("HttpResponse"), "HttpRequest" to orchestrator.resolve("HttpRequest"), + "HttpResponse" to orchestrator.resolve("HttpResponse"), "Interceptor" to interceptors.resolve("Interceptor"), "InterceptorContext" to interceptors.resolve("InterceptorContext"), "InterceptorError" to interceptors.resolve("error::InterceptorError"), - "ParamsBuilder" to endpointTypesGenerator.paramsBuilder(), + "Params" to endpointTypesGenerator.paramsStruct(), ) } fun render(writer: RustWriter, operationShape: OperationShape) { val operationName = symbolProvider.toSymbol(operationShape).name - renderInterceptor( - writer, - "${operationName}EndpointParamsInterceptor", - implInterceptorBodyForEndpointParams(operationShape), - ) - renderInterceptor( - writer, "${operationName}EndpointParamsFinalizerInterceptor", - implInterceptorBodyForEndpointParamsFinalizer, - ) - } - - private fun renderInterceptor(writer: RustWriter, interceptorName: String, implBody: Writable) { + val operationInput = symbolProvider.toSymbol(operationShape.inputShape(model)) + val interceptorName = "${operationName}EndpointParamsInterceptor" writer.rustTemplate( """ ##[derive(Debug)] @@ -68,37 +69,78 @@ class EndpointParamsInterceptorGenerator( context: &#{InterceptorContext}<#{HttpRequest}, #{HttpResponse}>, cfg: &mut #{ConfigBag}, ) -> Result<(), #{BoxError}> { - #{body:W} + let _input = context.input()?; + let _input = _input + .downcast_ref::<${operationInput.name}>() + .ok_or("failed to downcast to ${operationInput.name}")?; + + #{endpoint_prefix:W} + + // HACK: pull the handle out of the config bag until config is implemented right + let handle = cfg.get::>() + .expect("the handle is hacked into the config bag"); + let _config = &handle.conf; + + let params = #{Params}::builder() + #{param_setters} + .build() + .map_err(|err| #{ContextAttachedError}::new("endpoint params could not be built", err))?; + cfg.put(#{EndpointResolverParams}::new(params)); + Ok(()) } } """, *codegenScope, - "body" to implBody, + "endpoint_prefix" to endpointPrefix(operationShape), + "param_setters" to paramSetters(operationShape, endpointTypesGenerator.params), ) } - private fun implInterceptorBodyForEndpointParams(operationShape: OperationShape): Writable = writable { - val operationInput = symbolProvider.toSymbol(operationShape.inputShape(model)) - rustTemplate( - """ - let input = context.input()?; - let _input = input - .downcast_ref::<${operationInput.name}>() - .ok_or("failed to downcast to ${operationInput.name}")?; - let params_builder = cfg - .get::<#{ParamsBuilder}>() - .ok_or("missing endpoint params builder")? - .clone(); - ${"" /* TODO(EndpointResolver): Call setters on `params_builder` to update its fields by using values from `_input` */} - cfg.put(params_builder); + private fun paramSetters(operationShape: OperationShape, params: Parameters) = writable { + val idx = ContextIndex.of(codegenContext.model) + val memberParams = idx.getContextParams(operationShape).toList().sortedBy { it.first.memberName } + val builtInParams = params.toList().filter { it.isBuiltIn } + // first load builtins and their defaults + builtInParams.forEach { param -> + endpointTypesGenerator.builtInFor(param, "_config")?.also { defaultValue -> + rust(".set_${param.name.rustName()}(#W)", defaultValue) + } + } - #{endpoint_prefix:W} + idx.getClientContextParams(codegenContext.serviceShape).orNull()?.parameters?.forEach { (name, param) -> + val paramName = EndpointParamsGenerator.memberName(name) + val setterName = EndpointParamsGenerator.setterName(name) + if (param.type == ShapeType.BOOLEAN) { + rust(".$setterName(_config.$paramName)") + } else { + rust(".$setterName(_config.$paramName.clone())") + } + } - Ok(()) - """, - *codegenScope, - "endpoint_prefix" to endpointPrefix(operationShape), - ) + idx.getStaticContextParams(operationShape).orNull()?.parameters?.forEach { (name, param) -> + val setterName = EndpointParamsGenerator.setterName(name) + val value = param.value.toWritable() + rust(".$setterName(#W)", value) + } + + // lastly, allow these to be overridden by members + memberParams.forEach { (memberShape, param) -> + val memberName = codegenContext.symbolProvider.toMemberName(memberShape) + rust( + ".${EndpointParamsGenerator.setterName(param.name)}(_input.$memberName.clone())", + ) + } + } + + private fun Node.toWritable(): Writable { + val node = this + return writable { + when (node) { + is StringNode -> rust("Some(${node.value.dq()}.to_string())") + is BooleanNode -> rust("Some(${node.value})") + else -> PANIC("unsupported default value: $node") + } + } } private fun endpointPrefix(operationShape: OperationShape): Writable = writable { @@ -124,25 +166,4 @@ class EndpointParamsInterceptorGenerator( rust("cfg.put(endpoint_prefix);") } } - - private val implInterceptorBodyForEndpointParamsFinalizer: Writable = writable { - rustTemplate( - """ - let _ = context; - let params_builder = cfg - .get::<#{ParamsBuilder}>() - .ok_or("missing endpoint params builder")? - .clone(); - let params = params_builder - .build() - .map_err(|err| #{ContextAttachedError}::new("endpoint params could not be built", err))?; - cfg.put( - #{EndpointResolverParams}::new(params) - ); - - Ok(()) - """, - *codegenScope, - ) - } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index a570d4df91..dad89ae04e 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -70,7 +70,6 @@ class ServiceRuntimePluginGenerator( val runtimeApi = RuntimeType.smithyRuntimeApi(rc) arrayOf( "AnonymousIdentityResolver" to runtimeApi.resolve("client::identity::AnonymousIdentityResolver"), - "StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "BoxError" to runtimeApi.resolve("client::runtime_plugin::BoxError"), "ConfigBag" to runtimeApi.resolve("config_bag::ConfigBag"), "ConfigBagAccessors" to runtimeApi.resolve("client::orchestrator::ConfigBagAccessors"), @@ -85,6 +84,7 @@ class ServiceRuntimePluginGenerator( "ResolveEndpoint" to http.resolve("endpoint::ResolveEndpoint"), "RuntimePlugin" to runtimeApi.resolve("client::runtime_plugin::RuntimePlugin"), "SharedEndpointResolver" to http.resolve("endpoint::SharedEndpointResolver"), + "StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "TraceProbe" to runtimeApi.resolve("client::orchestrator::TraceProbe"), ) } @@ -106,6 +106,9 @@ class ServiceRuntimePluginGenerator( fn configure(&self, cfg: &mut #{ConfigBag}) -> Result<(), #{BoxError}> { use #{ConfigBagAccessors}; + // HACK: Put the handle into the config bag to work around config not being fully implemented yet + cfg.put(self.handle.clone()); + let http_auth_schemes = #{HttpAuthSchemes}::builder() #{http_auth_scheme_customizations} .build(); @@ -118,9 +121,6 @@ class ServiceRuntimePluginGenerator( #{SharedEndpointResolver}::from(self.handle.conf.endpoint_resolver())); cfg.set_endpoint_resolver(endpoint_resolver); - ${"" /* TODO(EndpointResolver): Create endpoint params builder from service config */} - cfg.put(#{Params}::builder()); - // TODO(RuntimePlugins): Wire up standard retry cfg.set_retry_strategy(#{NeverRetryStrategy}::new()); From 2fb21f6405d72045dc4e6ad6bdfbccf32ec00c0d Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Mon, 1 May 2023 12:01:06 -0400 Subject: [PATCH 06/10] split out interceptors (#2639) ## Motivation and Context - split out interceptors from config bag to simplify orchestrator ## Description - update runtime plugin trait to split out interceptors - remove interceptor generics - update interceptors struct to remove locking and indirection ## Testing - [x] ```./gradlew :aws:sra-test:assemble && (cd aws/sra-test/integration-tests/aws-sdk-s3 && cargo test)``` ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Zelda Hessler --- .../aws-runtime/src/invocation_id.rs | 11 +- .../aws-runtime/src/recursion_detection.rs | 5 +- .../aws-runtime/src/user_agent.rs | 10 +- .../benches/middleware_vs_orchestrator.rs | 2 + .../aws-sdk-s3/tests/sra_test.rs | 2 + .../EndpointParamsInterceptorGenerator.kt | 4 +- .../OperationRuntimePluginGenerator.kt | 10 +- .../ServiceRuntimePluginGenerator.kt | 11 +- .../config/ServiceConfigGenerator.kt | 3 +- .../src/client/interceptors.rs | 107 ++++-------------- .../src/client/interceptors/context.rs | 8 +- .../src/client/retries.rs | 4 +- .../src/client/runtime_plugin.rs | 38 +++++-- .../src/client/orchestrator.rs | 13 +-- .../src/client/orchestrator/endpoints.rs | 3 +- .../src/client/orchestrator/phase.rs | 23 ++-- .../src/client/retries/strategy/never.rs | 4 +- 17 files changed, 101 insertions(+), 157 deletions(-) diff --git a/aws/rust-runtime/aws-runtime/src/invocation_id.rs b/aws/rust-runtime/aws-runtime/src/invocation_id.rs index 55d2e981ac..89bcbb8d5b 100644 --- a/aws/rust-runtime/aws-runtime/src/invocation_id.rs +++ b/aws/rust-runtime/aws-runtime/src/invocation_id.rs @@ -5,7 +5,6 @@ use aws_smithy_runtime_api::client::interceptors::error::BoxError; use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext}; -use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::config_bag::ConfigBag; use http::{HeaderName, HeaderValue}; use uuid::Uuid; @@ -35,10 +34,10 @@ impl Default for InvocationIdInterceptor { } } -impl Interceptor for InvocationIdInterceptor { +impl Interceptor for InvocationIdInterceptor { fn modify_before_retry_loop( &self, - context: &mut InterceptorContext, + context: &mut InterceptorContext, _cfg: &mut ConfigBag, ) -> Result<(), BoxError> { let headers = context.request_mut()?.headers_mut(); @@ -74,15 +73,11 @@ mod tests { use crate::invocation_id::InvocationIdInterceptor; use aws_smithy_http::body::SdkBody; use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext}; - use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_smithy_runtime_api::type_erasure::TypedBox; use http::HeaderValue; - fn expect_header<'a>( - context: &'a InterceptorContext, - header_name: &str, - ) -> &'a HeaderValue { + fn expect_header<'a>(context: &'a InterceptorContext, header_name: &str) -> &'a HeaderValue { context .request() .unwrap() diff --git a/aws/rust-runtime/aws-runtime/src/recursion_detection.rs b/aws/rust-runtime/aws-runtime/src/recursion_detection.rs index c14f1d141b..deafdd973f 100644 --- a/aws/rust-runtime/aws-runtime/src/recursion_detection.rs +++ b/aws/rust-runtime/aws-runtime/src/recursion_detection.rs @@ -4,7 +4,6 @@ */ use aws_smithy_runtime_api::client::interceptors::{BoxError, Interceptor, InterceptorContext}; -use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_types::os_shim_internal::Env; use http::HeaderValue; @@ -37,10 +36,10 @@ impl RecursionDetectionInterceptor { } } -impl Interceptor for RecursionDetectionInterceptor { +impl Interceptor for RecursionDetectionInterceptor { fn modify_before_signing( &self, - context: &mut InterceptorContext, + context: &mut InterceptorContext, _cfg: &mut ConfigBag, ) -> Result<(), BoxError> { let request = context.request_mut()?; diff --git a/aws/rust-runtime/aws-runtime/src/user_agent.rs b/aws/rust-runtime/aws-runtime/src/user_agent.rs index efa8e3c0d5..3babb8c758 100644 --- a/aws/rust-runtime/aws-runtime/src/user_agent.rs +++ b/aws/rust-runtime/aws-runtime/src/user_agent.rs @@ -6,7 +6,6 @@ use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; use aws_smithy_runtime_api::client::interceptors::error::BoxError; use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext}; -use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_types::app_name::AppName; use aws_types::os_shim_internal::Env; @@ -70,10 +69,10 @@ fn header_values( )) } -impl Interceptor for UserAgentInterceptor { +impl Interceptor for UserAgentInterceptor { fn modify_before_signing( &self, - context: &mut InterceptorContext, + context: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), BoxError> { let api_metadata = cfg @@ -113,10 +112,7 @@ mod tests { use aws_smithy_runtime_api::type_erasure::TypedBox; use aws_smithy_types::error::display::DisplayErrorContext; - fn expect_header<'a>( - context: &'a InterceptorContext, - header_name: &str, - ) -> &'a str { + fn expect_header<'a>(context: &'a InterceptorContext, header_name: &str) -> &'a str { context .request() .unwrap() diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs index 622cdaca44..c6659cb7c1 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -6,6 +6,7 @@ #[macro_use] extern crate criterion; use aws_sdk_s3 as s3; +use aws_smithy_runtime_api::client::interceptors::Interceptors; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; use aws_smithy_runtime_api::config_bag::ConfigBag; use criterion::{BenchmarkId, Criterion}; @@ -94,6 +95,7 @@ async fn orchestrator(client: &s3::Client) { fn configure( &self, cfg: &mut ConfigBag, + _interceptors: &mut Interceptors, ) -> Result<(), aws_smithy_runtime_api::client::runtime_plugin::BoxError> { let params_builder = s3::endpoint::Params::builder() .set_region(Some(self.region.clone())) diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index bb7c5d4d31..6dd793e46b 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -10,6 +10,7 @@ use aws_sdk_s3::Client; use aws_smithy_client::dvr; use aws_smithy_client::dvr::MediaType; use aws_smithy_client::erase::DynConnector; +use aws_smithy_runtime_api::client::interceptors::Interceptors; use aws_smithy_runtime_api::client::orchestrator::{ConfigBagAccessors, RequestTime}; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; use aws_smithy_runtime_api::config_bag::ConfigBag; @@ -56,6 +57,7 @@ impl RuntimePlugin for FixupPlugin { fn configure( &self, cfg: &mut ConfigBag, + _interceptors: &mut Interceptors, ) -> Result<(), aws_smithy_runtime_api::client::runtime_plugin::BoxError> { cfg.set_request_time(RequestTime::new(self.timestamp.clone())); cfg.put(AwsUserAgent::for_tests()); diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt index d79ad7b600..4137857346 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt @@ -63,10 +63,10 @@ class EndpointParamsInterceptorGenerator( ##[derive(Debug)] struct $interceptorName; - impl #{Interceptor}<#{HttpRequest}, #{HttpResponse}> for $interceptorName { + impl #{Interceptor} for $interceptorName { fn read_before_execution( &self, - context: &#{InterceptorContext}<#{HttpRequest}, #{HttpResponse}>, + context: &#{InterceptorContext}, cfg: &mut #{ConfigBag}, ) -> Result<(), #{BoxError}> { let _input = context.input()?; diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt index a2ba50cb6f..45da1913dc 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt @@ -23,15 +23,14 @@ sealed class OperationRuntimePluginSection(name: String) : Section(name) { */ data class AdditionalConfig( val configBagName: String, + val interceptorName: String, val operationShape: OperationShape, ) : OperationRuntimePluginSection("AdditionalConfig") { fun registerInterceptor(runtimeConfig: RuntimeConfig, writer: RustWriter, interceptor: Writable) { val smithyRuntimeApi = RuntimeType.smithyRuntimeApi(runtimeConfig) writer.rustTemplate( """ - $configBagName.get::<#{Interceptors}<#{HttpRequest}, #{HttpResponse}>>() - .expect("interceptors set") - .register_operation_interceptor(std::sync::Arc::new(#{interceptor}) as _); + $interceptorName.register_operation_interceptor(std::sync::Arc::new(#{interceptor}) as _); """, "HttpRequest" to smithyRuntimeApi.resolve("client::orchestrator::HttpRequest"), "HttpResponse" to smithyRuntimeApi.resolve("client::orchestrator::HttpResponse"), @@ -83,6 +82,7 @@ class OperationRuntimePluginGenerator( "ConfigBagAccessors" to runtimeApi.resolve("client::orchestrator::ConfigBagAccessors"), "RetryClassifiers" to runtimeApi.resolve("client::retries::RetryClassifiers"), "RuntimePlugin" to runtimeApi.resolve("client::runtime_plugin::RuntimePlugin"), + "Interceptors" to runtimeApi.resolve("client::interceptors::Interceptors"), ) } @@ -95,7 +95,7 @@ class OperationRuntimePluginGenerator( writer.rustTemplate( """ impl #{RuntimePlugin} for $operationStructName { - fn configure(&self, cfg: &mut #{ConfigBag}) -> Result<(), #{BoxError}> { + fn configure(&self, cfg: &mut #{ConfigBag}, _interceptors: &mut #{Interceptors}) -> Result<(), #{BoxError}> { use #{ConfigBagAccessors} as _; cfg.set_request_serializer(${operationStructName}RequestSerializer); cfg.set_response_deserializer(${operationStructName}ResponseDeserializer); @@ -119,7 +119,7 @@ class OperationRuntimePluginGenerator( "additional_config" to writable { writeCustomizations( customizations, - OperationRuntimePluginSection.AdditionalConfig("cfg", operationShape), + OperationRuntimePluginSection.AdditionalConfig("cfg", "_interceptors", operationShape), ) }, "retry_classifier_customizations" to writable { diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index dad89ae04e..f35fe7446c 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -32,7 +32,7 @@ sealed class ServiceRuntimePluginSection(name: String) : Section(name) { /** * Hook for adding additional things to config inside service runtime plugins. */ - data class AdditionalConfig(val configBagName: String) : ServiceRuntimePluginSection("AdditionalConfig") { + data class AdditionalConfig(val configBagName: String, val interceptorName: String) : ServiceRuntimePluginSection("AdditionalConfig") { /** Adds a value to the config bag */ fun putConfigValue(writer: RustWriter, value: Writable) { writer.rust("$configBagName.put(#T);", value) @@ -43,9 +43,7 @@ sealed class ServiceRuntimePluginSection(name: String) : Section(name) { val smithyRuntimeApi = RuntimeType.smithyRuntimeApi(runtimeConfig) writer.rustTemplate( """ - $configBagName.get::<#{Interceptors}<#{HttpRequest}, #{HttpResponse}>>() - .expect("interceptors set") - .register_client_interceptor(std::sync::Arc::new(#{interceptor}) as _); + $interceptorName.register_client_interceptor(std::sync::Arc::new(#{interceptor}) as _); """, "HttpRequest" to smithyRuntimeApi.resolve("client::orchestrator::HttpRequest"), "HttpResponse" to smithyRuntimeApi.resolve("client::orchestrator::HttpResponse"), @@ -83,6 +81,7 @@ class ServiceRuntimePluginGenerator( "Params" to endpointTypesGenerator.paramsStruct(), "ResolveEndpoint" to http.resolve("endpoint::ResolveEndpoint"), "RuntimePlugin" to runtimeApi.resolve("client::runtime_plugin::RuntimePlugin"), + "Interceptors" to runtimeApi.resolve("client::interceptors::Interceptors"), "SharedEndpointResolver" to http.resolve("endpoint::SharedEndpointResolver"), "StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"), "TraceProbe" to runtimeApi.resolve("client::orchestrator::TraceProbe"), @@ -103,7 +102,7 @@ class ServiceRuntimePluginGenerator( } impl #{RuntimePlugin} for ServiceRuntimePlugin { - fn configure(&self, cfg: &mut #{ConfigBag}) -> Result<(), #{BoxError}> { + fn configure(&self, cfg: &mut #{ConfigBag}, _interceptors: &mut #{Interceptors}) -> Result<(), #{BoxError}> { use #{ConfigBagAccessors}; // HACK: Put the handle into the config bag to work around config not being fully implemented yet @@ -154,7 +153,7 @@ class ServiceRuntimePluginGenerator( writeCustomizations(customizations, ServiceRuntimePluginSection.HttpAuthScheme("cfg")) }, "additional_config" to writable { - writeCustomizations(customizations, ServiceRuntimePluginSection.AdditionalConfig("cfg")) + writeCustomizations(customizations, ServiceRuntimePluginSection.AdditionalConfig("cfg", "_interceptors")) }, ) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt index bb0653f327..39ecaf164e 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt @@ -294,7 +294,7 @@ class ServiceConfigGenerator(private val customizations: List Result<(), #{BoxError}> { + fn configure(&self, _cfg: &mut #{ConfigBag}, _inter: &mut #{Interceptors}) -> Result<(), #{BoxError}> { // TODO(RuntimePlugins): Put into `cfg` the fields in `self.config_override` that are not `None`. Ok(()) @@ -302,6 +302,7 @@ class ServiceConfigGenerator(private val customizations: List { #[doc = $docs] - fn $name( - &self, - context: &InterceptorContext, - cfg: &mut ConfigBag, - ) -> Result<(), BoxError> { + fn $name(&self, context: &InterceptorContext, cfg: &mut ConfigBag) -> Result<(), BoxError> { let _ctx = context; let _cfg = cfg; Ok(()) @@ -29,7 +25,7 @@ macro_rules! interceptor_trait_fn { #[doc = $docs] fn $name( &self, - context: &mut InterceptorContext, + context: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), BoxError> { let _ctx = context; @@ -49,7 +45,7 @@ macro_rules! interceptor_trait_fn { /// of the SDK ’s request execution pipeline. Hooks are either "read" hooks, which make it possible /// to read in-flight request or response messages, or "read/write" hooks, which make it possible /// to modify in-flight request or output messages. -pub trait Interceptor: std::fmt::Debug { +pub trait Interceptor: std::fmt::Debug { interceptor_trait_fn!( read_before_execution, " @@ -541,47 +537,12 @@ pub trait Interceptor: std::fmt::Debug { ); } -pub type SharedInterceptor = Arc + Send + Sync>; - -#[derive(Debug)] -struct Inner { - client_interceptors: Vec>, - operation_interceptors: Vec>, -} - -// The compiler isn't smart enough to realize that TxReq and TxRes don't need to implement `Clone` -impl Clone for Inner { - fn clone(&self) -> Self { - Self { - client_interceptors: self.client_interceptors.clone(), - operation_interceptors: self.operation_interceptors.clone(), - } - } -} - -#[derive(Debug)] -pub struct Interceptors { - inner: Arc>>, -} +pub type SharedInterceptor = Arc; -// The compiler isn't smart enough to realize that TxReq and TxRes don't need to implement `Clone` -impl Clone for Interceptors { - fn clone(&self) -> Self { - Self { - inner: self.inner.clone(), - } - } -} - -impl Default for Interceptors { - fn default() -> Self { - Self { - inner: Arc::new(Mutex::new(Inner { - client_interceptors: Vec::new(), - operation_interceptors: Vec::new(), - })), - } - } +#[derive(Debug, Clone, Default)] +pub struct Interceptors { + client_interceptors: Vec, + operation_interceptors: Vec, } macro_rules! interceptor_impl_fn { @@ -592,16 +553,10 @@ macro_rules! interceptor_impl_fn { interceptor_impl_fn!(mut context, $name, $name); }; (context, $outer_name:ident, $inner_name:ident) => { - interceptor_impl_fn!( - $outer_name, - $inner_name(context: &InterceptorContext) - ); + interceptor_impl_fn!($outer_name, $inner_name(context: &InterceptorContext)); }; (mut context, $outer_name:ident, $inner_name:ident) => { - interceptor_impl_fn!( - $outer_name, - $inner_name(context: &mut InterceptorContext) - ); + interceptor_impl_fn!($outer_name, $inner_name(context: &mut InterceptorContext)); }; ($outer_name:ident, $inner_name:ident ($context:ident : $context_ty:ty)) => { pub fn $outer_name( @@ -623,48 +578,26 @@ macro_rules! interceptor_impl_fn { }; } -impl Interceptors { +impl Interceptors { pub fn new() -> Self { Self::default() } - fn interceptors(&self) -> Vec> { + fn interceptors(&self) -> impl Iterator { // Since interceptors can modify the interceptor list (since its in the config bag), copy the list ahead of time. // This should be cheap since the interceptors inside the list are Arcs. - // TODO(enableNewSmithyRuntime): Remove the ability for interceptors to modify the interceptor list and then simplify this - let mut interceptors = self.inner.lock().unwrap().client_interceptors.clone(); - interceptors.extend( - self.inner - .lock() - .unwrap() - .operation_interceptors - .iter() - .cloned(), - ); - interceptors + self.client_interceptors + .iter() + .chain(self.operation_interceptors.iter()) } - pub fn register_client_interceptor( - &self, - interceptor: SharedInterceptor, - ) -> &Self { - self.inner - .lock() - .unwrap() - .client_interceptors - .push(interceptor); + pub fn register_client_interceptor(&mut self, interceptor: SharedInterceptor) -> &mut Self { + self.client_interceptors.push(interceptor); self } - pub fn register_operation_interceptor( - &self, - interceptor: SharedInterceptor, - ) -> &Self { - self.inner - .lock() - .unwrap() - .operation_interceptors - .push(interceptor); + pub fn register_operation_interceptor(&mut self, interceptor: SharedInterceptor) -> &mut Self { + self.operation_interceptors.push(interceptor); self } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs index 60e6461fb8..61d7c7fd5d 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs @@ -4,6 +4,7 @@ */ use super::InterceptorError; +use crate::client::orchestrator::{HttpRequest, HttpResponse}; use crate::type_erasure::TypeErasedBox; pub type Input = TypeErasedBox; @@ -11,8 +12,11 @@ pub type Output = TypeErasedBox; pub type Error = TypeErasedBox; pub type OutputOrError = Result; +type Request = HttpRequest; +type Response = HttpResponse; + /// A container for the data currently available to an interceptor. -pub struct InterceptorContext { +pub struct InterceptorContext { input: Option, output_or_error: Option, request: Option, @@ -21,7 +25,7 @@ pub struct InterceptorContext { // TODO(interceptors) we could use types to ensure that people calling methods on interceptor context can't access // field that haven't been set yet. -impl InterceptorContext { +impl InterceptorContext { pub fn new(input: Input) -> Self { Self { input: Some(input), diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs b/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs index a53f1d242c..1331317afe 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/retries.rs @@ -5,7 +5,7 @@ use crate::client::interceptors::context::Error; use crate::client::interceptors::InterceptorContext; -use crate::client::orchestrator::{BoxError, HttpRequest, HttpResponse}; +use crate::client::orchestrator::BoxError; use crate::config_bag::ConfigBag; use aws_smithy_types::retry::ErrorKind; use std::fmt::Debug; @@ -23,7 +23,7 @@ pub trait RetryStrategy: Send + Sync + Debug { fn should_attempt_retry( &self, - context: &InterceptorContext, + context: &InterceptorContext, cfg: &ConfigBag, ) -> Result; } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs index 835686f337..8c2604700b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs @@ -3,17 +3,26 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::client::interceptors::Interceptors; use crate::config_bag::ConfigBag; pub type BoxError = Box; pub trait RuntimePlugin { - fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError>; + fn configure( + &self, + cfg: &mut ConfigBag, + interceptors: &mut Interceptors, + ) -> Result<(), BoxError>; } impl RuntimePlugin for Box { - fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { - self.as_ref().configure(cfg) + fn configure( + &self, + cfg: &mut ConfigBag, + interceptors: &mut Interceptors, + ) -> Result<(), BoxError> { + self.as_ref().configure(cfg, interceptors) } } @@ -38,17 +47,25 @@ impl RuntimePlugins { self } - pub fn apply_client_configuration(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { + pub fn apply_client_configuration( + &self, + cfg: &mut ConfigBag, + interceptors: &mut Interceptors, + ) -> Result<(), BoxError> { for plugin in self.client_plugins.iter() { - plugin.configure(cfg)?; + plugin.configure(cfg, interceptors)?; } Ok(()) } - pub fn apply_operation_configuration(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { + pub fn apply_operation_configuration( + &self, + cfg: &mut ConfigBag, + interceptors: &mut Interceptors, + ) -> Result<(), BoxError> { for plugin in self.operation_plugins.iter() { - plugin.configure(cfg)?; + plugin.configure(cfg, interceptors)?; } Ok(()) @@ -58,12 +75,17 @@ impl RuntimePlugins { #[cfg(test)] mod tests { use super::{BoxError, RuntimePlugin, RuntimePlugins}; + use crate::client::interceptors::Interceptors; use crate::config_bag::ConfigBag; struct SomeStruct; impl RuntimePlugin for SomeStruct { - fn configure(&self, _cfg: &mut ConfigBag) -> Result<(), BoxError> { + fn configure( + &self, + _cfg: &mut ConfigBag, + _inters: &mut Interceptors, + ) -> Result<(), BoxError> { todo!() } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index adba3dfb04..7dfef49dde 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -10,9 +10,7 @@ use crate::client::orchestrator::phase::Phase; use aws_smithy_http::result::SdkError; use aws_smithy_runtime_api::client::interceptors::context::{Error, Input, Output}; use aws_smithy_runtime_api::client::interceptors::{InterceptorContext, Interceptors}; -use aws_smithy_runtime_api::client::orchestrator::{ - BoxError, ConfigBagAccessors, HttpRequest, HttpResponse, -}; +use aws_smithy_runtime_api::client::orchestrator::{BoxError, ConfigBagAccessors, HttpResponse}; use aws_smithy_runtime_api::client::retries::ShouldAttempt; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins; use aws_smithy_runtime_api::config_bag::ConfigBag; @@ -31,15 +29,14 @@ pub async fn invoke( let mut cfg = ConfigBag::base(); let cfg = &mut cfg; - let interceptors = Interceptors::new(); - cfg.put(interceptors.clone()); + let mut interceptors = Interceptors::new(); let context = Phase::construction(InterceptorContext::new(input)) // Client configuration - .include(|_| runtime_plugins.apply_client_configuration(cfg))? + .include(|_| runtime_plugins.apply_client_configuration(cfg, &mut interceptors))? .include(|ctx| interceptors.client_read_before_execution(ctx, cfg))? // Operation configuration - .include(|_| runtime_plugins.apply_operation_configuration(cfg))? + .include(|_| runtime_plugins.apply_operation_configuration(cfg, &mut interceptors))? .include(|ctx| interceptors.operation_read_before_execution(ctx, cfg))? // Before serialization .include(|ctx| interceptors.read_before_serialization(ctx, cfg))? @@ -117,7 +114,7 @@ pub async fn invoke( async fn make_an_attempt( dispatch_phase: Phase, cfg: &mut ConfigBag, - interceptors: &Interceptors, + interceptors: &Interceptors, ) -> Result> { let dispatch_phase = dispatch_phase .include(|ctx| interceptors.read_before_attempt(ctx, cfg))? diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs index 981ef3239c..89570c1ef6 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs @@ -10,7 +10,6 @@ use aws_smithy_http::endpoint::{ use aws_smithy_runtime_api::client::interceptors::InterceptorContext; use aws_smithy_runtime_api::client::orchestrator::{ BoxError, ConfigBagAccessors, EndpointResolver, EndpointResolverParams, HttpRequest, - HttpResponse, }; use aws_smithy_runtime_api::config_bag::ConfigBag; use http::header::HeaderName; @@ -113,7 +112,7 @@ where } pub(super) fn orchestrate_endpoint( - ctx: &mut InterceptorContext, + ctx: &mut InterceptorContext, cfg: &ConfigBag, ) -> Result<(), BoxError> { let params = cfg.endpoint_resolver_params(); diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/phase.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/phase.rs index f8af01c4b5..9e101050b7 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/phase.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/phase.rs @@ -6,7 +6,7 @@ use aws_smithy_http::result::{ConnectorError, SdkError}; use aws_smithy_runtime_api::client::interceptors::context::{Error, Output}; use aws_smithy_runtime_api::client::interceptors::InterceptorContext; -use aws_smithy_runtime_api::client::orchestrator::{BoxError, HttpRequest, HttpResponse}; +use aws_smithy_runtime_api::client::orchestrator::{BoxError, HttpResponse}; #[derive(Copy, Clone, Eq, PartialEq)] enum OrchestrationPhase { @@ -17,26 +17,21 @@ enum OrchestrationPhase { pub(super) struct Phase { phase: OrchestrationPhase, - context: InterceptorContext, + context: InterceptorContext, } impl Phase { - pub(crate) fn construction(context: InterceptorContext) -> Self { + pub(crate) fn construction(context: InterceptorContext) -> Self { Self::start(OrchestrationPhase::Construction, context) } - pub(crate) fn dispatch(context: InterceptorContext) -> Self { + pub(crate) fn dispatch(context: InterceptorContext) -> Self { Self::start(OrchestrationPhase::Dispatch, context) } - pub(crate) fn response_handling( - context: InterceptorContext, - ) -> Self { + pub(crate) fn response_handling(context: InterceptorContext) -> Self { Self::start(OrchestrationPhase::ResponseHandling, context) } - fn start( - phase: OrchestrationPhase, - context: InterceptorContext, - ) -> Self { + fn start(phase: OrchestrationPhase, context: InterceptorContext) -> Self { match phase { OrchestrationPhase::Construction => {} OrchestrationPhase::Dispatch => {} @@ -47,7 +42,7 @@ impl Phase { pub(crate) fn include_mut>( mut self, - c: impl FnOnce(&mut InterceptorContext) -> Result<(), E>, + c: impl FnOnce(&mut InterceptorContext) -> Result<(), E>, ) -> Result> { match c(&mut self.context) { Ok(_) => Ok(self), @@ -57,7 +52,7 @@ impl Phase { pub(crate) fn include>( self, - c: impl FnOnce(&InterceptorContext) -> Result<(), E>, + c: impl FnOnce(&InterceptorContext) -> Result<(), E>, ) -> Result> { match c(&self.context) { Ok(_) => Ok(self), @@ -113,7 +108,7 @@ impl Phase { } } - pub(crate) fn finish(self) -> InterceptorContext { + pub(crate) fn finish(self) -> InterceptorContext { self.context } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/never.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/never.rs index 49366a273d..f415af3691 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/never.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/never.rs @@ -4,7 +4,7 @@ */ use aws_smithy_runtime_api::client::interceptors::InterceptorContext; -use aws_smithy_runtime_api::client::orchestrator::{BoxError, HttpRequest, HttpResponse}; +use aws_smithy_runtime_api::client::orchestrator::BoxError; use aws_smithy_runtime_api::client::retries::{RetryStrategy, ShouldAttempt}; use aws_smithy_runtime_api::config_bag::ConfigBag; @@ -24,7 +24,7 @@ impl RetryStrategy for NeverRetryStrategy { fn should_attempt_retry( &self, - _context: &InterceptorContext, + _context: &InterceptorContext, _cfg: &ConfigBag, ) -> Result { Ok(ShouldAttempt::No) From d3957a6ee9b50517991946d9f557832097699797 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 1 May 2023 16:51:00 -0500 Subject: [PATCH 07/10] Allow canary-runner to specify expected speech text by Transcribe (#2660) ## Motivation and Context During a release for `aws-sdk-rust`, we've found that the canary test received an improved speech text from Transcribe. This broke the canary test because the test hard-coded the string that should be expected by Transcribe. This PR updates the runner so that the expected speech text can be passed-in from outside of the program. ## Testing Added unit tests, mainly for argument parsing. ## Checklist If the next release is created off of the main branch (because it's a non-breaking release), we only need to merge this PR the main branch. If we need to release from a release branch at some point in the future, this PR will need to be duplicated and merged to the release branch as well. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito --- tools/ci-cdk/canary-runner/src/run.rs | 104 +++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 12 deletions(-) diff --git a/tools/ci-cdk/canary-runner/src/run.rs b/tools/ci-cdk/canary-runner/src/run.rs index 5d03cd44fd..b5e0058d74 100644 --- a/tools/ci-cdk/canary-runner/src/run.rs +++ b/tools/ci-cdk/canary-runner/src/run.rs @@ -83,6 +83,11 @@ pub struct RunArgs { #[clap(long)] musl: bool, + /// Expected speech text generated by Transcribe. This needs to be passed-in + /// because it can change as the accuracy of generated text improves over time. + #[clap(long)] + expected_speech_text_by_transcribe: Option, + /// File path to a CDK outputs JSON file. This can be used instead /// of all the --lambda... args. #[clap(long)] @@ -101,12 +106,13 @@ pub struct RunArgs { lambda_execution_role_arn: Option, } -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] struct Options { rust_version: Option, sdk_release_tag: Option, sdk_path: Option, musl: bool, + expected_speech_text_by_transcribe: Option, lambda_code_s3_bucket_name: String, lambda_test_s3_bucket_name: String, lambda_execution_role_arn: String, @@ -139,6 +145,7 @@ impl Options { sdk_release_tag: run_opt.sdk_release_tag, sdk_path: run_opt.sdk_path, musl: run_opt.musl, + expected_speech_text_by_transcribe: run_opt.expected_speech_text_by_transcribe, lambda_code_s3_bucket_name: value.inner.lambda_code_s3_bucket_name, lambda_test_s3_bucket_name: value.inner.lambda_test_s3_bucket_name, lambda_execution_role_arn: value.inner.lambda_execution_role_arn, @@ -149,6 +156,7 @@ impl Options { sdk_release_tag: run_opt.sdk_release_tag, sdk_path: run_opt.sdk_path, musl: run_opt.musl, + expected_speech_text_by_transcribe: run_opt.expected_speech_text_by_transcribe, lambda_code_s3_bucket_name: run_opt.lambda_code_s3_bucket_name.expect("required"), lambda_test_s3_bucket_name: run_opt.lambda_test_s3_bucket_name.expect("required"), lambda_execution_role_arn: run_opt.lambda_execution_role_arn.expect("required"), @@ -250,6 +258,7 @@ async fn run_canary(options: &Options, config: &aws_config::SdkConfig) -> Result bundle_name, bundle_file_name, &options.lambda_execution_role_arn, + options.expected_speech_text_by_transcribe.as_ref(), &options.lambda_code_s3_bucket_name, &options.lambda_test_s3_bucket_name, ) @@ -324,11 +333,27 @@ async fn create_lambda_fn( bundle_name: &str, bundle_file_name: &str, execution_role: &str, + expected_speech_text_by_transcribe: Option<&String>, code_s3_bucket: &str, test_s3_bucket: &str, ) -> Result<()> { use lambda::model::*; + let env_builder = match expected_speech_text_by_transcribe { + Some(expected_speech_text_by_transcribe) => Environment::builder() + .variables("RUST_BACKTRACE", "1") + .variables("RUST_LOG", "info") + .variables("CANARY_S3_BUCKET_NAME", test_s3_bucket) + .variables( + "CANARY_EXPECTED_TRANSCRIBE_RESULT", + expected_speech_text_by_transcribe, + ), + None => Environment::builder() + .variables("RUST_BACKTRACE", "1") + .variables("RUST_LOG", "info") + .variables("CANARY_S3_BUCKET_NAME", test_s3_bucket), + }; + lambda_client .create_function() .function_name(bundle_name) @@ -342,17 +367,7 @@ async fn create_lambda_fn( .build(), ) .publish(true) - .environment( - Environment::builder() - .variables("RUST_BACKTRACE", "1") - .variables("RUST_LOG", "info") - .variables("CANARY_S3_BUCKET_NAME", test_s3_bucket) - .variables( - "CANARY_EXPECTED_TRANSCRIBE_RESULT", - "Good day to you transcribe. This is Polly talking to you from the Rust ST K.", - ) - .build(), - ) + .environment(env_builder.build()) .timeout(60) .send() .await @@ -419,3 +434,68 @@ async fn delete_lambda_fn(lambda_client: lambda::Client, bundle_name: &str) -> R .context(here!("failed to delete Lambda"))?; Ok(()) } + +#[cfg(test)] +mod tests { + use crate::run::Options; + use crate::run::RunArgs; + use clap::Parser; + + #[test] + fn args_parsing() { + assert_eq!( + RunArgs { + rust_version: None, + sdk_release_tag: None, + sdk_path: Some("artifact-aws-sdk-rust/sdk".into()), + musl: false, + expected_speech_text_by_transcribe: Some("Good day to you transcribe.".to_owned()), + cdk_output: Some("../cdk-outputs.json".into()), + lambda_code_s3_bucket_name: None, + lambda_test_s3_bucket_name: None, + lambda_execution_role_arn: None + }, + RunArgs::try_parse_from([ + "run", + "--sdk-path", + "artifact-aws-sdk-rust/sdk", + "--expected-speech-text-by-transcribe", + "Good day to you transcribe.", + "--cdk-output", + "../cdk-outputs.json", + ]) + .unwrap() + ); + } + + #[test] + fn options_from_args() { + let run_args = RunArgs::try_parse_from([ + "run", + "--sdk-path", + "artifact-aws-sdk-rust/sdk", + "--expected-speech-text-by-transcribe", + "Good day to you transcribe.", + "--lambda-code-s3-bucket-name", + "bucket-for-code", + "--lambda-test-s3-bucket-name", + "bucket-for-test", + "--lambda-execution-role-arn", + "arn:aws:lambda::role/exe-role", + ]) + .unwrap(); + assert_eq!( + Options { + rust_version: None, + sdk_release_tag: None, + sdk_path: Some("artifact-aws-sdk-rust/sdk".into()), + musl: false, + expected_speech_text_by_transcribe: Some("Good day to you transcribe.".to_owned()), + lambda_code_s3_bucket_name: "bucket-for-code".to_owned(), + lambda_test_s3_bucket_name: "bucket-for-test".to_owned(), + lambda_execution_role_arn: "arn:aws:lambda::role/exe-role".to_owned(), + }, + Options::load_from(run_args).unwrap(), + ); + } +} From bf23d57aa3551b49d49165a84db788cee1529509 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Tue, 2 May 2023 10:00:49 -0400 Subject: [PATCH 08/10] Adds an experimental store/load add to config bag (#2654) ## Motivation and Context - store multiple objects of the same type in the bag - obtain mutable access to items in the bag ## Description This adds `store_append` to enable storing multiple items of the same type in the bag and `get_mut`, `get_mut_or_else` and `get_mut_or_default` to reduce clones when possible ## Testing UTs ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../aws-smithy-runtime-api/src/config_bag.rs | 547 ++++++++++++++---- .../src/config_bag/typeid_map.rs | 32 + 2 files changed, 482 insertions(+), 97 deletions(-) create mode 100644 rust-runtime/aws-smithy-runtime-api/src/config_bag/typeid_map.rs diff --git a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs index 418fbd9b13..530c8da317 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs @@ -9,9 +9,18 @@ //! with the following properties: //! 1. A new layer of configuration may be applied onto an existing configuration structure without modifying it or taking ownership. //! 2. No lifetime shenanigans to deal with -use aws_smithy_http::property_bag::PropertyBag; +mod typeid_map; + +use crate::config_bag::typeid_map::TypeIdMap; + +use std::any::{type_name, Any, TypeId}; +use std::borrow::Cow; + use std::fmt::{Debug, Formatter}; +use std::iter::Rev; +use std::marker::PhantomData; use std::ops::Deref; +use std::slice; use std::sync::Arc; /// Layered Configuration Structure @@ -28,14 +37,7 @@ impl Debug for ConfigBag { struct Layers<'a>(&'a ConfigBag); impl Debug for Layers<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let mut list = f.debug_list(); - list.entry(&self.0.head); - let mut us = self.0.tail.as_ref(); - while let Some(bag) = us { - list.entry(&bag.head); - us = bag.tail.as_ref() - } - list.finish() + f.debug_list().entries(self.0.layers()).finish() } } f.debug_struct("ConfigBag") @@ -59,42 +61,201 @@ impl Deref for FrozenConfigBag { } } -pub trait Persist { - fn layer_name(&self) -> &'static str; - fn persist(&self, layer: &mut ConfigBag); +/// Private module to keep Value type while avoiding "private type in public latest" +pub(crate) mod value { + #[derive(Debug)] + pub enum Value { + Set(T), + ExplicitlyUnset(&'static str), + } +} +use value::Value; + +impl Default for Value { + fn default() -> Self { + Self::Set(Default::default()) + } +} + +struct DebugErased { + field: Box, + #[allow(dead_code)] + type_name: &'static str, + #[allow(clippy::type_complexity)] + debug: Box) -> std::fmt::Result + Send + Sync>, +} + +impl Debug for DebugErased { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + (self.debug)(self, f) + } +} + +impl DebugErased { + fn new(value: T) -> Self { + let debug = |value: &DebugErased, f: &mut Formatter<'_>| { + Debug::fmt(value.as_ref::().expect("typechecked"), f) + }; + let name = type_name::(); + Self { + field: Box::new(value), + type_name: name, + debug: Box::new(debug), + } + } + + fn as_ref(&self) -> Option<&T> { + self.field.downcast_ref() + } + + fn as_mut(&mut self) -> Option<&mut T> { + self.field.downcast_mut() + } +} + +pub struct Layer { + name: Cow<'static, str>, + props: TypeIdMap, +} + +/// Trait defining how types can be stored and loaded from the config bag +pub trait Store: Sized + Send + Sync + 'static { + type ReturnedType<'a>: Send + Sync; + type StoredType: Send + Sync + Debug; + + /// Create a returned type from an iterable of items + fn merge_iter(iter: ItemIter<'_, Self>) -> Self::ReturnedType<'_>; +} + +/// Store an item in the config bag by replacing the existing value +#[non_exhaustive] +pub struct StoreReplace(PhantomData); + +/// Store an item in the config bag by effectively appending it to a list +#[non_exhaustive] +pub struct StoreAppend(PhantomData); + +pub trait Storable: Send + Sync + Debug + 'static { + type Storer: Store; } -pub trait Load: Sized { - fn load(bag: &ConfigBag) -> Option; +impl Store for StoreReplace { + type ReturnedType<'a> = Option<&'a U>; + type StoredType = Value; + + fn merge_iter(mut iter: ItemIter<'_, Self>) -> Self::ReturnedType<'_> { + iter.next().and_then(|item| match item { + Value::Set(item) => Some(item), + Value::ExplicitlyUnset(_) => None, + }) + } } -pub trait ConfigLayer: Persist + Load {} +impl Store for StoreAppend { + type ReturnedType<'a> = AppendItemIter<'a, U>; + type StoredType = Value>; + + fn merge_iter(iter: ItemIter<'_, Self>) -> Self::ReturnedType<'_> { + AppendItemIter { + inner: iter, + cur: None, + } + } +} -enum Value { - Set(T), - ExplicitlyUnset, +/// Iterator of items returned by [`StoreAppend`] +pub struct AppendItemIter<'a, U> { + inner: ItemIter<'a, StoreAppend>, + cur: Option>>, } -struct Layer { - name: &'static str, - props: PropertyBag, +impl<'a, U: 'a> Iterator for AppendItemIter<'a, U> +where + U: Send + Sync + Debug + 'static, +{ + type Item = &'a U; + + fn next(&mut self) -> Option { + if let Some(buf) = &mut self.cur { + match buf.next() { + Some(item) => return Some(item), + None => self.cur = None, + } + } + match self.inner.next() { + None => None, + Some(Value::Set(u)) => { + self.cur = Some(u.iter().rev()); + self.next() + } + Some(Value::ExplicitlyUnset(_)) => None, + } + } } impl Debug for Layer { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - struct Contents<'a>(&'a Layer); - impl Debug for Contents<'_> { + struct Items<'a>(&'a Layer); + impl Debug for Items<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.debug_list().entries(self.0.props.contents()).finish() + f.debug_list().entries(self.0.props.values()).finish() } } f.debug_struct("Layer") .field("name", &self.name) - .field("properties", &Contents(self)) + .field("items", &Items(self)) .finish() } } +impl Layer { + pub fn put(&mut self, value: T::StoredType) -> &mut Self { + self.props + .insert(TypeId::of::(), DebugErased::new(value)); + self + } + + pub fn get(&self) -> Option<&T::StoredType> { + self.props + .get(&TypeId::of::()) + .map(|t| t.as_ref().expect("typechecked")) + } + + pub fn get_mut(&mut self) -> Option<&mut T::StoredType> { + self.props + .get_mut(&TypeId::of::()) + .map(|t| t.as_mut().expect("typechecked")) + } + + pub fn get_mut_or_default(&mut self) -> &mut T::StoredType + where + T::StoredType: Default, + { + self.props + .entry(TypeId::of::()) + .or_insert_with(|| DebugErased::new(T::StoredType::default())) + .as_mut() + .expect("typechecked") + } + + pub fn is_empty(&self) -> bool { + self.props.is_empty() + } + + pub fn len(&self) -> usize { + self.props.len() + } +} + +pub trait Accessor { + type Setter: Setter; + fn config(&self) -> &ConfigBag; +} + +pub trait Setter { + fn config(&mut self) -> &mut ConfigBag; +} + fn no_op(_: &mut ConfigBag) {} impl FrozenConfigBag { @@ -119,19 +280,19 @@ impl FrozenConfigBag { /// add_more_config(&mut bag); /// let bag = bag.freeze(); /// ``` - pub fn add_layer(&self, name: &'static str) -> ConfigBag { + pub fn add_layer(&self, name: impl Into>) -> ConfigBag { self.with_fn(name, no_op) } - pub fn with(&self, layer: impl Persist) -> ConfigBag { - self.with_fn(layer.layer_name(), |bag| layer.persist(bag)) - } - /// Add more items to the config bag - pub fn with_fn(&self, name: &'static str, next: impl Fn(&mut ConfigBag)) -> ConfigBag { + pub fn with_fn( + &self, + name: impl Into>, + next: impl Fn(&mut ConfigBag), + ) -> ConfigBag { let new_layer = Layer { - name, - props: PropertyBag::new(), + name: name.into(), + props: Default::default(), }; let mut bag = ConfigBag { head: new_layer, @@ -146,29 +307,154 @@ impl ConfigBag { pub fn base() -> Self { ConfigBag { head: Layer { - name: "base", + name: Cow::Borrowed("base"), props: Default::default(), }, tail: None, } } + pub fn store_put(&mut self, item: T) -> &mut Self + where + T: Storable>, + { + self.head.put::>(Value::Set(item)); + self + } + + pub fn store_or_unset(&mut self, item: Option) -> &mut Self + where + T: Storable>, + { + let item = match item { + Some(item) => Value::Set(item), + None => Value::ExplicitlyUnset(type_name::()), + }; + self.head.put::>(item); + self + } + + /// This can only be used for types that use [`StoreAppend`] + /// ``` + /// use aws_smithy_runtime_api::config_bag::{ConfigBag, Storable, StoreAppend, StoreReplace}; + /// let mut bag = ConfigBag::base(); + /// #[derive(Debug, PartialEq, Eq)] + /// struct Interceptor(&'static str); + /// impl Storable for Interceptor { + /// type Storer = StoreAppend; + /// } + /// + /// bag.store_append(Interceptor("123")); + /// bag.store_append(Interceptor("456")); + /// + /// assert_eq!( + /// bag.load::().collect::>(), + /// vec![&Interceptor("456"), &Interceptor("123")] + /// ); + /// ``` + pub fn store_append(&mut self, item: T) -> &mut Self + where + T: Storable>, + { + match self.head.get_mut_or_default::>() { + Value::Set(list) => list.push(item), + v @ Value::ExplicitlyUnset(_) => *v = Value::Set(vec![item]), + } + self + } + + pub fn clear(&mut self) + where + T: Storable>, + { + self.head + .put::>(Value::ExplicitlyUnset(type_name::())); + } + + pub fn load(&self) -> ::ReturnedType<'_> { + self.sourced_get::() + } + /// Retrieve the value of type `T` from the bag if exists pub fn get(&self) -> Option<&T> { - let mut source = vec![]; - let out = self.sourced_get(&mut source); + let out = self.sourced_get::>(); out } + /// Returns a mutable reference to `T` if it is stored in the top layer of the bag + pub fn get_mut(&mut self) -> Option<&mut T> + where + T: Storable>, + { + // this code looks weird to satisfy the borrow checker—we can't keep the result of `get_mut` + // alive (even in a returned branch) and then call `store_put`. So: drop the borrow immediately + // store, the value, then pull it right back + if matches!(self.head.get_mut::>(), None) { + let new_item = match self.tail.as_deref().and_then(|b| b.load::()) { + Some(item) => item.clone(), + None => return None, + }; + self.store_put(new_item); + self.get_mut() + } else if matches!( + self.head.get::>(), + Some(Value::ExplicitlyUnset(_)) + ) { + None + } else if let Some(Value::Set(t)) = self.head.get_mut::>() { + Some(t) + } else { + unreachable!() + } + } + + /// Returns a mutable reference to `T` if it is stored in the top layer of the bag + /// + /// - If `T` is in a deeper layer of the bag, that value will be cloned and inserted into the top layer + /// - If `T` is not present in the bag, the [`Default`] implementation will be used. + pub fn get_mut_or_default( + &mut self, + ) -> &mut T + where + T: Storable>, + { + self.get_mut_or_else(|| T::default()) + } + + /// Returns a mutable reference to `T` if it is stored in the top layer of the bag + /// + /// - If `T` is in a deeper layer of the bag, that value will be cloned and inserted into the top layer + /// - If `T` is not present in the bag, `default` will be used to construct a new value + pub fn get_mut_or_else( + &mut self, + default: impl Fn() -> T, + ) -> &mut T + where + T: Storable>, + { + // this code looks weird to satisfy the borrow checker—we can't keep the result of `get_mut` + // alive (even in a returned branch) and then call `store_put`. So: drop the borrow immediately + // store, the value, then pull it right back + if self.get_mut::().is_none() { + self.store_put((default)()); + return self + .get_mut() + .expect("item was just stored in the top layer"); + } + // above it was None + self.get_mut().unwrap() + } + /// Insert `value` into the bag pub fn put(&mut self, value: T) -> &mut Self { - self.head.props.insert(Value::Set(value)); + self.head.put::>(Value::Set(value)); self } /// Remove `T` from this bag - pub fn unset(&mut self) -> &mut Self { - self.head.props.insert(Value::::ExplicitlyUnset); + pub fn unset(&mut self) -> &mut Self { + self.head + .put::>(Value::ExplicitlyUnset(type_name::())); self } @@ -200,37 +486,56 @@ impl ConfigBag { self.freeze().with_fn(name, next) } - pub fn with(self, layer: impl Persist) -> ConfigBag { - self.freeze().with(layer) - } - - pub fn add_layer(self, name: &'static str) -> ConfigBag { + pub fn add_layer(self, name: impl Into>) -> ConfigBag { self.freeze().add_layer(name) } - pub fn sourced_get( - &self, - source_trail: &mut Vec, - ) -> Option<&T> { - // todo: optimize so we don't need to compute the source if it's unused - let bag = &self.head; - let inner_item = self - .tail - .as_ref() - .and_then(|bag| bag.sourced_get(source_trail)); - let (item, source) = match bag.props.get::>() { - Some(Value::ExplicitlyUnset) => (None, SourceInfo::Unset { layer: bag.name }), - Some(Value::Set(v)) => ( - Some(v), - SourceInfo::Set { - layer: bag.name, - value: format!("{:?}", v), - }, - ), - None => (inner_item, SourceInfo::Inherit { layer: bag.name }), + pub fn sourced_get(&self) -> T::ReturnedType<'_> { + let stored_type_iter = ItemIter { + inner: self.layers(), + t: PhantomData::default(), }; - source_trail.push(source); - item + T::merge_iter(stored_type_iter) + } + + fn layers(&self) -> BagIter<'_> { + BagIter { bag: Some(self) } + } +} + +/// Iterator of items returned from config_bag +pub struct ItemIter<'a, T> { + inner: BagIter<'a>, + t: PhantomData, +} +impl<'a, T: 'a> Iterator for ItemIter<'a, T> +where + T: Store, +{ + type Item = &'a T::StoredType; + + fn next(&mut self) -> Option { + match self.inner.next() { + Some(layer) => layer.get::().or_else(|| self.next()), + None => None, + } + } +} + +/// Iterator over the layers of a config bag +struct BagIter<'a> { + bag: Option<&'a ConfigBag>, +} + +impl<'a> Iterator for BagIter<'a> { + type Item = &'a Layer; + + fn next(&mut self) -> Option { + let next = self.bag.map(|b| &b.head); + if let Some(bag) = &mut self.bag { + self.bag = bag.tail.as_deref(); + } + next } } @@ -250,7 +555,7 @@ pub enum SourceInfo { #[cfg(test)] mod test { use super::ConfigBag; - use crate::config_bag::{Load, Persist}; + use crate::config_bag::{Storable, StoreAppend, StoreReplace}; #[test] fn layered_property_bag() { @@ -318,47 +623,95 @@ mod test { let mut open_bag = operation_config.with_fn("my_custom_info", |_bag: &mut ConfigBag| {}); open_bag.put("foo"); + + assert_eq!(open_bag.layers().count(), 4); } #[test] - fn persist_trait() { - #[derive(Debug, Eq, PartialEq, Clone)] - struct MyConfig { - a: bool, - b: String, + fn store_append() { + let mut bag = ConfigBag::base(); + #[derive(Debug, PartialEq, Eq)] + struct Interceptor(&'static str); + impl Storable for Interceptor { + type Storer = StoreAppend; } - #[derive(Debug)] - struct A(bool); - #[derive(Debug)] - struct B(String); - - impl Persist for MyConfig { - fn layer_name(&self) -> &'static str { - "my_config" - } + bag.clear::(); + // you can only call store_append because interceptor is marked with a vec + bag.store_append(Interceptor("123")); + bag.store_append(Interceptor("456")); + + let mut bag = bag.add_layer("next"); + bag.store_append(Interceptor("789")); + + assert_eq!( + bag.load::().collect::>(), + vec![ + &Interceptor("789"), + &Interceptor("456"), + &Interceptor("123") + ] + ); + + bag.clear::(); + assert_eq!(bag.load::().count(), 0); + } - fn persist(&self, layer: &mut ConfigBag) { - layer.put(A(self.a)); - layer.put(B(self.b.clone())); - } + #[test] + fn store_append_many_layers() { + #[derive(Debug, PartialEq, Eq, Clone)] + struct TestItem(i32, i32); + impl Storable for TestItem { + type Storer = StoreAppend; } - impl Load for MyConfig { - fn load(bag: &ConfigBag) -> Option { - Some(MyConfig { - a: bag.get::().unwrap().0, - b: bag.get::().unwrap().0.clone(), - }) + let mut expected = vec![]; + let mut bag = ConfigBag::base(); + for layer in 0..100 { + bag = bag.add_layer(format!("{}", layer)); + for item in 0..100 { + expected.push(TestItem(layer, item)); + bag.store_append(TestItem(layer, item)); } } + expected.reverse(); + assert_eq!( + bag.load::() + .map(|i| i.clone()) + .collect::>(), + expected + ); + } - let conf = MyConfig { - a: true, - b: "hello!".to_string(), - }; + #[test] + fn get_mut_or_else() { + #[derive(Clone, Debug, PartialEq, Eq, Default)] + struct Foo(usize); + impl Storable for Foo { + type Storer = StoreReplace; + } + + let mut bag = ConfigBag::base(); + assert_eq!(bag.get_mut::(), None); + assert_eq!(bag.get_mut_or_default::(), &Foo(0)); + bag.get_mut_or_default::().0 += 1; + assert_eq!(bag.get::(), Some(&Foo(1))); - let bag = ConfigBag::base().with(conf.clone()); + let bag = bag.freeze(); - assert_eq!(MyConfig::load(&bag), Some(conf)); + let old_ref = bag.load::().unwrap(); + assert_eq!(old_ref, &Foo(1)); + + // there is one in the bag, so it can be returned + let mut next = bag.add_layer("next"); + next.get_mut::().unwrap().0 += 1; + let new_ref = next.load::().unwrap(); + assert_eq!(new_ref, &Foo(2)); + // no funny business + assert_eq!(old_ref, &Foo(1)); + + next.unset::(); + // if it was unset, we can't clone the current one, that would be wrong + assert_eq!(next.get_mut::(), None); + assert_eq!(next.get_mut_or_default::(), &Foo(0)); } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/config_bag/typeid_map.rs b/rust-runtime/aws-smithy-runtime-api/src/config_bag/typeid_map.rs new file mode 100644 index 0000000000..68818c9b58 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/src/config_bag/typeid_map.rs @@ -0,0 +1,32 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use std::any::TypeId; +use std::collections::HashMap; +use std::hash::{BuildHasherDefault, Hasher}; + +pub(super) type TypeIdMap = HashMap>; + +// With TypeIds as keys, there's no need to hash them. They are already hashes +// themselves, coming from the compiler. The IdHasher just holds the u64 of +// the TypeId, and then returns it, instead of doing any bit fiddling. +#[derive(Default)] +pub(super) struct IdHasher(u64); + +impl Hasher for IdHasher { + #[inline] + fn finish(&self) -> u64 { + self.0 + } + + fn write(&mut self, _: &[u8]) { + unreachable!("TypeId calls write_u64"); + } + + #[inline] + fn write_u64(&mut self, id: u64) { + self.0 = id; + } +} From 23d0f5ec67317c86d7ad29db6f6cb5ec5961ad58 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 2 May 2023 10:56:57 -0700 Subject: [PATCH 09/10] Implement timeouts in the orchestrator (#2661) ## Motivation and Context This PR implements timeouts in the orchestrator. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../s3/tests/alternative-async-runtime.rs | 8 +- .../integration-tests/s3/tests/timeouts.rs | 7 +- .../ResiliencyConfigCustomization.kt | 21 ++ .../customize/RequiredCustomizations.kt | 8 + rust-runtime/aws-smithy-async/src/lib.rs | 5 +- .../src/client/orchestrator.rs | 17 ++ rust-runtime/aws-smithy-runtime/Cargo.toml | 5 +- rust-runtime/aws-smithy-runtime/src/client.rs | 2 + .../src/client/orchestrator.rs | 27 ++ .../aws-smithy-runtime/src/client/timeout.rs | 240 ++++++++++++++++++ 10 files changed, 335 insertions(+), 5 deletions(-) create mode 100644 rust-runtime/aws-smithy-runtime/src/client/timeout.rs diff --git a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs index 209403de4a..7d6e84a90f 100644 --- a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs +++ b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs @@ -15,6 +15,7 @@ use aws_smithy_async::assert_elapsed; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_client::never::NeverConnector; use aws_smithy_http::result::SdkError; +use aws_smithy_types::error::display::DisplayErrorContext; use aws_smithy_types::timeout::TimeoutConfig; use std::fmt::Debug; use std::sync::Arc; @@ -117,7 +118,12 @@ async fn timeout_test(sleep_impl: Arc) -> Result<(), Box, + ): List = + baseCustomizations + listOf(ResiliencyServiceRuntimePluginCustomization()) } diff --git a/rust-runtime/aws-smithy-async/src/lib.rs b/rust-runtime/aws-smithy-async/src/lib.rs index 7e106da5db..f9c4f9d9d9 100644 --- a/rust-runtime/aws-smithy-async/src/lib.rs +++ b/rust-runtime/aws-smithy-async/src/lib.rs @@ -37,12 +37,13 @@ macro_rules! assert_elapsed { ($start:expr, $dur:expr, $margin_of_error:expr) => {{ let elapsed = $start.elapsed(); // type ascription improves compiler error when wrong type is passed - let lower: std::time::Duration = $dur; let margin_of_error: std::time::Duration = $margin_of_error; + let lower: std::time::Duration = $dur - margin_of_error; + let upper: std::time::Duration = $dur + margin_of_error; // Handles ms rounding assert!( - elapsed >= lower && elapsed <= lower + margin_of_error, + elapsed >= lower && elapsed <= upper, "actual = {:?}, expected = {:?}", elapsed, lower diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs index db36e762d9..a8ebb01aab 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs @@ -11,12 +11,14 @@ use crate::client::retries::RetryStrategy; use crate::config_bag::ConfigBag; use crate::type_erasure::{TypeErasedBox, TypedBox}; use aws_smithy_async::future::now_or_later::NowOrLater; +use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::EndpointPrefix; use std::any::Any; use std::fmt::Debug; use std::future::Future as StdFuture; use std::pin::Pin; +use std::sync::Arc; use std::time::SystemTime; pub type HttpRequest = http::Request; @@ -142,6 +144,9 @@ pub trait ConfigBagAccessors { fn request_time(&self) -> Option; fn set_request_time(&mut self, request_time: RequestTime); + + fn sleep_impl(&self) -> Option>; + fn set_sleep_impl(&mut self, async_sleep: Option>); } impl ConfigBagAccessors for ConfigBag { @@ -276,4 +281,16 @@ impl ConfigBagAccessors for ConfigBag { fn set_request_time(&mut self, request_time: RequestTime) { self.put::(request_time); } + + fn sleep_impl(&self) -> Option> { + self.get::>().cloned() + } + + fn set_sleep_impl(&mut self, sleep_impl: Option>) { + if let Some(sleep_impl) = sleep_impl { + self.put::>(sleep_impl); + } else { + self.unset::>(); + } + } } diff --git a/rust-runtime/aws-smithy-runtime/Cargo.toml b/rust-runtime/aws-smithy-runtime/Cargo.toml index 5c8e098462..79e5c7f9fe 100644 --- a/rust-runtime/aws-smithy-runtime/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime/Cargo.toml @@ -14,6 +14,7 @@ http-auth = ["aws-smithy-runtime-api/http-auth"] test-util = ["dep:aws-smithy-protocol-test"] [dependencies] +aws-smithy-async = { path = "../aws-smithy-async" } aws-smithy-client = { path = "../aws-smithy-client" } aws-smithy-http = { path = "../aws-smithy-http" } aws-smithy-protocol-test = { path = "../aws-smithy-protocol-test", optional = true } @@ -22,12 +23,14 @@ aws-smithy-types = { path = "../aws-smithy-types" } bytes = "1" http = "0.2.8" http-body = "0.4.5" +pin-project-lite = "0.2.7" pin-utils = "0.1.0" tokio = { version = "1.25", features = [] } tracing = "0.1" [dev-dependencies] -tokio = { version = "1.25", features = ["macros", "rt"] } +aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] } +tokio = { version = "1.25", features = ["macros", "rt", "test-util"] } [package.metadata.docs.rs] all-features = true diff --git a/rust-runtime/aws-smithy-runtime/src/client.rs b/rust-runtime/aws-smithy-runtime/src/client.rs index d477ce9eae..14d1c95ac1 100644 --- a/rust-runtime/aws-smithy-runtime/src/client.rs +++ b/rust-runtime/aws-smithy-runtime/src/client.rs @@ -15,3 +15,5 @@ pub mod connections; /// This code defines when and how failed requests should be retried. It also defines the behavior /// used to limit the rate at which requests are sent. pub mod retries; + +mod timeout; diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index 7dfef49dde..c0f4b4f06b 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -7,6 +7,7 @@ use self::auth::orchestrate_auth; use crate::client::orchestrator::endpoints::orchestrate_endpoint; use crate::client::orchestrator::http::read_body; use crate::client::orchestrator::phase::Phase; +use crate::client::timeout::{MaybeTimeout, ProvideMaybeTimeoutConfig, TimeoutKind}; use aws_smithy_http::result::SdkError; use aws_smithy_runtime_api::client::interceptors::context::{Error, Input, Output}; use aws_smithy_runtime_api::client::interceptors::{InterceptorContext, Interceptors}; @@ -25,6 +26,15 @@ pub(self) mod phase; pub async fn invoke( input: Input, runtime_plugins: &RuntimePlugins, +) -> Result> { + invoke_pre_config(input, runtime_plugins) + .instrument(debug_span!("invoke")) + .await +} + +async fn invoke_pre_config( + input: Input, + runtime_plugins: &RuntimePlugins, ) -> Result> { let mut cfg = ConfigBag::base(); let cfg = &mut cfg; @@ -38,6 +48,20 @@ pub async fn invoke( // Operation configuration .include(|_| runtime_plugins.apply_operation_configuration(cfg, &mut interceptors))? .include(|ctx| interceptors.operation_read_before_execution(ctx, cfg))? + .finish(); + + let operation_timeout_config = cfg.maybe_timeout_config(TimeoutKind::Operation); + invoke_post_config(cfg, context, interceptors) + .maybe_timeout_with_config(operation_timeout_config) + .await +} + +async fn invoke_post_config( + cfg: &mut ConfigBag, + context: InterceptorContext, + interceptors: Interceptors, +) -> Result> { + let context = Phase::construction(context) // Before serialization .include(|ctx| interceptors.read_before_serialization(ctx, cfg))? .include_mut(|ctx| interceptors.modify_before_serialization(ctx, cfg))? @@ -76,8 +100,11 @@ pub async fn invoke( let mut context = context; let handling_phase = loop { + let attempt_timeout_config = cfg.maybe_timeout_config(TimeoutKind::OperationAttempt); let dispatch_phase = Phase::dispatch(context); context = make_an_attempt(dispatch_phase, cfg, &interceptors) + .instrument(debug_span!("make_an_attempt")) + .maybe_timeout_with_config(attempt_timeout_config) .await? .include(|ctx| interceptors.read_after_attempt(ctx, cfg))? .include_mut(|ctx| interceptors.modify_before_attempt_completion(ctx, cfg))? diff --git a/rust-runtime/aws-smithy-runtime/src/client/timeout.rs b/rust-runtime/aws-smithy-runtime/src/client/timeout.rs new file mode 100644 index 0000000000..66c43d8eb3 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime/src/client/timeout.rs @@ -0,0 +1,240 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use aws_smithy_async::future::timeout::Timeout; +use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; +use aws_smithy_client::SdkError; +use aws_smithy_runtime_api::client::orchestrator::{ConfigBagAccessors, HttpResponse}; +use aws_smithy_runtime_api::config_bag::ConfigBag; +use aws_smithy_types::timeout::TimeoutConfig; +use pin_project_lite::pin_project; +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; +use std::time::Duration; + +#[derive(Debug)] +struct MaybeTimeoutError { + kind: TimeoutKind, + duration: Duration, +} + +impl MaybeTimeoutError { + fn new(kind: TimeoutKind, duration: Duration) -> Self { + Self { kind, duration } + } +} + +impl std::fmt::Display for MaybeTimeoutError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{} occurred after {:?}", + match self.kind { + TimeoutKind::Operation => "operation timeout (all attempts including retries)", + TimeoutKind::OperationAttempt => "operation attempt timeout (single attempt)", + }, + self.duration + ) + } +} + +impl std::error::Error for MaybeTimeoutError {} + +pin_project! { + #[non_exhaustive] + #[must_use = "futures do nothing unless you `.await` or poll them"] + // This allow is needed because otherwise Clippy will get mad we didn't document the + // generated MaybeTimeoutFutureProj + #[allow(missing_docs)] + #[project = MaybeTimeoutFutureProj] + /// A timeout future that may or may not have a timeout depending on + /// whether or not one was set. A `kind` can be set so that when a timeout occurs, there + /// is additional context attached to the error. + pub(super) enum MaybeTimeoutFuture { + /// A wrapper around an inner future that will output an [`SdkError`] if it runs longer than + /// the given duration + Timeout { + #[pin] + future: Timeout, + timeout_kind: TimeoutKind, + duration: Duration, + }, + /// A thin wrapper around an inner future that will never time out + NoTimeout { + #[pin] + future: F + } + } +} + +impl Future for MaybeTimeoutFuture +where + InnerFuture: Future>>, +{ + type Output = Result>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let (future, kind, duration) = match self.project() { + MaybeTimeoutFutureProj::NoTimeout { future } => return future.poll(cx), + MaybeTimeoutFutureProj::Timeout { + future, + timeout_kind, + duration, + } => (future, timeout_kind, duration), + }; + match future.poll(cx) { + Poll::Ready(Ok(response)) => Poll::Ready(response), + Poll::Ready(Err(_timeout)) => Poll::Ready(Err(SdkError::timeout_error( + MaybeTimeoutError::new(*kind, *duration), + ))), + Poll::Pending => Poll::Pending, + } + } +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) enum TimeoutKind { + Operation, + OperationAttempt, +} + +#[derive(Clone, Debug)] +pub(super) struct MaybeTimeoutConfig { + sleep_impl: Option>, + timeout: Option, + timeout_kind: TimeoutKind, +} + +pub(super) trait ProvideMaybeTimeoutConfig { + fn maybe_timeout_config(&self, timeout_kind: TimeoutKind) -> MaybeTimeoutConfig; +} + +impl ProvideMaybeTimeoutConfig for ConfigBag { + fn maybe_timeout_config(&self, timeout_kind: TimeoutKind) -> MaybeTimeoutConfig { + if let Some(timeout_config) = self.get::() { + let sleep_impl = self.sleep_impl(); + let timeout = match (sleep_impl.as_ref(), timeout_kind) { + (None, _) => None, + (Some(_), TimeoutKind::Operation) => timeout_config.operation_timeout(), + (Some(_), TimeoutKind::OperationAttempt) => { + timeout_config.operation_attempt_timeout() + } + }; + MaybeTimeoutConfig { + sleep_impl, + timeout, + timeout_kind, + } + } else { + MaybeTimeoutConfig { + sleep_impl: None, + timeout: None, + timeout_kind, + } + } + } +} + +/// Trait to conveniently wrap a future with an optional timeout. +pub(super) trait MaybeTimeout: Sized { + /// Wraps a future in a timeout if one is set. + fn maybe_timeout_with_config( + self, + timeout_config: MaybeTimeoutConfig, + ) -> MaybeTimeoutFuture; + + /// Wraps a future in a timeout if one is set. + fn maybe_timeout(self, cfg: &ConfigBag, kind: TimeoutKind) -> MaybeTimeoutFuture; +} + +impl MaybeTimeout for T +where + T: Future, +{ + fn maybe_timeout_with_config( + self, + timeout_config: MaybeTimeoutConfig, + ) -> MaybeTimeoutFuture { + match timeout_config { + MaybeTimeoutConfig { + sleep_impl: Some(sleep_impl), + timeout: Some(timeout), + timeout_kind, + } => MaybeTimeoutFuture::Timeout { + future: Timeout::new(self, sleep_impl.sleep(timeout)), + timeout_kind, + duration: timeout, + }, + _ => MaybeTimeoutFuture::NoTimeout { future: self }, + } + } + + fn maybe_timeout(self, cfg: &ConfigBag, kind: TimeoutKind) -> MaybeTimeoutFuture { + self.maybe_timeout_with_config(cfg.maybe_timeout_config(kind)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use aws_smithy_async::assert_elapsed; + use aws_smithy_async::future::never::Never; + use aws_smithy_async::rt::sleep::TokioSleep; + + #[tokio::test] + async fn test_no_timeout() { + let sleep_impl: Arc = Arc::new(TokioSleep::new()); + let sleep_future = sleep_impl.sleep(Duration::from_millis(250)); + let underlying_future = async { + sleep_future.await; + Result::<_, SdkError<(), HttpResponse>>::Ok(()) + }; + + let now = tokio::time::Instant::now(); + tokio::time::pause(); + + let mut cfg = ConfigBag::base(); + cfg.put(TimeoutConfig::builder().build()); + cfg.set_sleep_impl(Some(sleep_impl)); + + underlying_future + .maybe_timeout(&cfg, TimeoutKind::Operation) + .await + .expect("success"); + + assert_elapsed!(now, Duration::from_secs_f32(0.25)); + } + + #[tokio::test] + async fn test_operation_timeout() { + let sleep_impl: Arc = Arc::new(TokioSleep::new()); + let never = Never::new(); + let underlying_future = async { + never.await; + Result::<_, SdkError<(), HttpResponse>>::Ok(()) + }; + + let now = tokio::time::Instant::now(); + tokio::time::pause(); + + let mut cfg = ConfigBag::base(); + cfg.put( + TimeoutConfig::builder() + .operation_timeout(Duration::from_millis(250)) + .build(), + ); + cfg.set_sleep_impl(Some(sleep_impl)); + + let result = underlying_future + .maybe_timeout(&cfg, TimeoutKind::Operation) + .await; + let err = result.expect_err("should have timed out"); + + assert_eq!(format!("{:?}", err), "TimeoutError(TimeoutError { source: MaybeTimeoutError { kind: Operation, duration: 250ms } })"); + assert_elapsed!(now, Duration::from_secs_f32(0.25)); + } +} From e1cf72e52d152519f57a66d95a23322fe1de0759 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 2 May 2023 14:37:47 -0700 Subject: [PATCH 10/10] Make the `Debug` impl on `TypeErasedBox` useful (#2665) ## Motivation and Context This PR ports @rcoh's addition of `Debug` in `ConfigBag` to the `TypeErasedBox`. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../protocol/ResponseDeserializerGenerator.kt | 4 +- .../aws-smithy-runtime-api/src/client/auth.rs | 13 ++- .../src/client/orchestrator.rs | 17 ++- .../aws-smithy-runtime-api/src/config_bag.rs | 53 ++------- .../src/type_erasure.rs | 102 ++++++++++-------- 5 files changed, 82 insertions(+), 107 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt index 81c0494c17..4b355cf8ba 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt @@ -156,8 +156,8 @@ class ResponseDeserializerGenerator( """ pub(crate) fn $fnName(result: Result) -> Result<#{Output}, #{Error}> where - O: Send + Sync + 'static, - E: Send + Sync + 'static, + O: std::fmt::Debug + Send + Sync + 'static, + E: std::fmt::Debug + Send + Sync + 'static, { result.map(|output| #{TypedBox}::new(output).erase()) .map_err(|error| #{TypedBox}::new(error).erase()) diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs b/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs index 3d8cccad47..fefc3c5025 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/auth.rs @@ -7,9 +7,8 @@ use crate::client::identity::{Identity, IdentityResolver, IdentityResolvers}; use crate::client::orchestrator::{BoxError, HttpRequest}; use crate::config_bag::ConfigBag; use crate::type_erasure::{TypeErasedBox, TypedBox}; -use std::any::Any; use std::borrow::Cow; -use std::fmt::Debug; +use std::fmt; use std::sync::Arc; #[cfg(feature = "http-auth")] @@ -39,16 +38,16 @@ impl AuthSchemeId { pub struct AuthOptionResolverParams(TypeErasedBox); impl AuthOptionResolverParams { - pub fn new(params: T) -> Self { + pub fn new(params: T) -> Self { Self(TypedBox::new(params).erase()) } - pub fn get(&self) -> Option<&T> { + pub fn get(&self) -> Option<&T> { self.0.downcast_ref() } } -pub trait AuthOptionResolver: Send + Sync + Debug { +pub trait AuthOptionResolver: Send + Sync + fmt::Debug { fn resolve_auth_options<'a>( &'a self, params: &AuthOptionResolverParams, @@ -87,7 +86,7 @@ impl HttpAuthSchemes { } } -pub trait HttpAuthScheme: Send + Sync + Debug { +pub trait HttpAuthScheme: Send + Sync + fmt::Debug { fn scheme_id(&self) -> AuthSchemeId; fn identity_resolver<'a>( @@ -98,7 +97,7 @@ pub trait HttpAuthScheme: Send + Sync + Debug { fn request_signer(&self) -> &dyn HttpRequestSigner; } -pub trait HttpRequestSigner: Send + Sync + Debug { +pub trait HttpRequestSigner: Send + Sync + fmt::Debug { /// Return a signed version of the given request using the given identity. /// /// If the provided identity is incompatible with this signer, an error must be returned. diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs index a8ebb01aab..30751d2120 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs @@ -14,8 +14,7 @@ use aws_smithy_async::future::now_or_later::NowOrLater; use aws_smithy_async::rt::sleep::AsyncSleep; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::EndpointPrefix; -use std::any::Any; -use std::fmt::Debug; +use std::fmt; use std::future::Future as StdFuture; use std::pin::Pin; use std::sync::Arc; @@ -27,15 +26,15 @@ pub type BoxError = Box; pub type BoxFuture = Pin>>>; pub type Future = NowOrLater, BoxFuture>; -pub trait TraceProbe: Send + Sync + Debug { +pub trait TraceProbe: Send + Sync + fmt::Debug { fn dispatch_events(&self); } -pub trait RequestSerializer: Send + Sync + Debug { +pub trait RequestSerializer: Send + Sync + fmt::Debug { fn serialize_input(&self, input: Input) -> Result; } -pub trait ResponseDeserializer: Send + Sync + Debug { +pub trait ResponseDeserializer: Send + Sync + fmt::Debug { fn deserialize_streaming(&self, response: &mut HttpResponse) -> Option { let _ = response; None @@ -44,7 +43,7 @@ pub trait ResponseDeserializer: Send + Sync + Debug { fn deserialize_nonstreaming(&self, response: &HttpResponse) -> OutputOrError; } -pub trait Connection: Send + Sync + Debug { +pub trait Connection: Send + Sync + fmt::Debug { fn call(&self, request: HttpRequest) -> BoxFuture; } @@ -58,16 +57,16 @@ impl Connection for Box { pub struct EndpointResolverParams(TypeErasedBox); impl EndpointResolverParams { - pub fn new(params: T) -> Self { + pub fn new(params: T) -> Self { Self(TypedBox::new(params).erase()) } - pub fn get(&self) -> Option<&T> { + pub fn get(&self) -> Option<&T> { self.0.downcast_ref() } } -pub trait EndpointResolver: Send + Sync + Debug { +pub trait EndpointResolver: Send + Sync + fmt::Debug { fn resolve_and_apply_endpoint( &self, params: &EndpointResolverParams, diff --git a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs index 530c8da317..78a28538fb 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs @@ -12,10 +12,9 @@ mod typeid_map; use crate::config_bag::typeid_map::TypeIdMap; - -use std::any::{type_name, Any, TypeId}; +use crate::type_erasure::TypeErasedBox; +use std::any::{type_name, TypeId}; use std::borrow::Cow; - use std::fmt::{Debug, Formatter}; use std::iter::Rev; use std::marker::PhantomData; @@ -77,45 +76,9 @@ impl Default for Value { } } -struct DebugErased { - field: Box, - #[allow(dead_code)] - type_name: &'static str, - #[allow(clippy::type_complexity)] - debug: Box) -> std::fmt::Result + Send + Sync>, -} - -impl Debug for DebugErased { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - (self.debug)(self, f) - } -} - -impl DebugErased { - fn new(value: T) -> Self { - let debug = |value: &DebugErased, f: &mut Formatter<'_>| { - Debug::fmt(value.as_ref::().expect("typechecked"), f) - }; - let name = type_name::(); - Self { - field: Box::new(value), - type_name: name, - debug: Box::new(debug), - } - } - - fn as_ref(&self) -> Option<&T> { - self.field.downcast_ref() - } - - fn as_mut(&mut self) -> Option<&mut T> { - self.field.downcast_mut() - } -} - pub struct Layer { name: Cow<'static, str>, - props: TypeIdMap, + props: TypeIdMap, } /// Trait defining how types can be stored and loaded from the config bag @@ -211,20 +174,20 @@ impl Debug for Layer { impl Layer { pub fn put(&mut self, value: T::StoredType) -> &mut Self { self.props - .insert(TypeId::of::(), DebugErased::new(value)); + .insert(TypeId::of::(), TypeErasedBox::new(value)); self } pub fn get(&self) -> Option<&T::StoredType> { self.props .get(&TypeId::of::()) - .map(|t| t.as_ref().expect("typechecked")) + .map(|t| t.downcast_ref().expect("typechecked")) } pub fn get_mut(&mut self) -> Option<&mut T::StoredType> { self.props .get_mut(&TypeId::of::()) - .map(|t| t.as_mut().expect("typechecked")) + .map(|t| t.downcast_mut().expect("typechecked")) } pub fn get_mut_or_default(&mut self) -> &mut T::StoredType @@ -233,8 +196,8 @@ impl Layer { { self.props .entry(TypeId::of::()) - .or_insert_with(|| DebugErased::new(T::StoredType::default())) - .as_mut() + .or_insert_with(|| TypeErasedBox::new(T::StoredType::default())) + .downcast_mut() .expect("typechecked") } diff --git a/rust-runtime/aws-smithy-runtime-api/src/type_erasure.rs b/rust-runtime/aws-smithy-runtime-api/src/type_erasure.rs index 50c852f0cf..4c20c6c4d4 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/type_erasure.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/type_erasure.rs @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -use std::any::Any; +use std::any::{type_name, Any}; +use std::fmt; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; @@ -18,7 +19,6 @@ use std::ops::{Deref, DerefMut}; /// and to avoid the monomorphization that brings with it. This `TypedBox` will primarily be useful /// for operation-specific or service-specific interceptors that need to operate on the actual /// input/output/error types. -#[derive(Debug)] pub struct TypedBox { inner: TypeErasedBox, _phantom: PhantomData, @@ -26,12 +26,12 @@ pub struct TypedBox { impl TypedBox where - T: Send + Sync + 'static, + T: fmt::Debug + Send + Sync + 'static, { // Creates a new `TypedBox`. pub fn new(inner: T) -> Self { Self { - inner: TypeErasedBox::new(Box::new(inner) as _), + inner: TypeErasedBox::new(inner), _phantom: Default::default(), } } @@ -62,7 +62,17 @@ where } } -impl Deref for TypedBox { +impl fmt::Debug for TypedBox +where + T: Send + Sync + 'static, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("TypedBox:")?; + (self.inner.debug)(&self.inner, f) + } +} + +impl Deref for TypedBox { type Target = T; fn deref(&self) -> &Self::Target { @@ -70,67 +80,66 @@ impl Deref for TypedBox { } } -impl DerefMut for TypedBox { +impl DerefMut for TypedBox { fn deref_mut(&mut self) -> &mut Self::Target { self.inner.downcast_mut().expect("type checked") } } -#[derive(Debug)] -pub struct TypedRef<'a, T> { - inner: &'a TypeErasedBox, - _phantom: PhantomData, -} - -impl<'a, T: 'static> TypedRef<'a, T> { - pub fn assume_from(type_erased: &'a TypeErasedBox) -> Option> { - if type_erased.downcast_ref::().is_some() { - Some(TypedRef { - inner: type_erased, - _phantom: Default::default(), - }) - } else { - None - } - } +/// A new-type around `Box` +pub struct TypeErasedBox { + field: Box, + #[allow(dead_code)] + type_name: &'static str, + #[allow(clippy::type_complexity)] + debug: Box) -> fmt::Result + Send + Sync>, } -impl<'a, T: 'static> Deref for TypedRef<'a, T> { - type Target = T; - - fn deref(&self) -> &Self::Target { - self.inner.downcast_ref().expect("type checked") +impl fmt::Debug for TypeErasedBox { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("TypeErasedBox:")?; + (self.debug)(self, f) } } -/// A new-type around `Box` -#[derive(Debug)] -pub struct TypeErasedBox { - inner: Box, -} - impl TypeErasedBox { - // Creates a new `TypeErasedBox`. - pub fn new(inner: Box) -> Self { - Self { inner } + pub fn new(value: T) -> Self { + let debug = |value: &TypeErasedBox, f: &mut fmt::Formatter<'_>| { + fmt::Debug::fmt(value.downcast_ref::().expect("typechecked"), f) + }; + let name = type_name::(); + Self { + field: Box::new(value), + type_name: name, + debug: Box::new(debug), + } } // Downcast into a `Box`, or return `Self` if it is not a `T`. - pub fn downcast(self) -> Result, Self> { - match self.inner.downcast() { + pub fn downcast(self) -> Result, Self> { + let TypeErasedBox { + field, + type_name, + debug, + } = self; + match field.downcast() { Ok(t) => Ok(t), - Err(s) => Err(Self { inner: s }), + Err(s) => Err(Self { + field: s, + type_name, + debug, + }), } } /// Downcast as a `&T`, or return `None` if it is not a `T`. - pub fn downcast_ref(&self) -> Option<&T> { - self.inner.downcast_ref() + pub fn downcast_ref(&self) -> Option<&T> { + self.field.downcast_ref() } /// Downcast as a `&mut T`, or return `None` if it is not a `T`. - pub fn downcast_mut(&mut self) -> Option<&mut T> { - self.inner.downcast_mut() + pub fn downcast_mut(&mut self) -> Option<&mut T> { + self.field.downcast_mut() } } @@ -148,6 +157,9 @@ mod tests { let foo = TypedBox::new(Foo("1")); let bar = TypedBox::new(Bar(2)); + assert_eq!("TypedBox:Foo(\"1\")", format!("{foo:?}")); + assert_eq!("TypedBox:Bar(2)", format!("{bar:?}")); + let mut foo_erased = foo.erase(); foo_erased .downcast_mut::() @@ -155,6 +167,8 @@ mod tests { .0 = "3"; let bar_erased = bar.erase(); + assert_eq!("TypeErasedBox:Foo(\"3\")", format!("{foo_erased:?}")); + assert_eq!("TypeErasedBox:Bar(2)", format!("{bar_erased:?}")); let bar_erased = TypedBox::::assume_from(bar_erased).expect_err("it's not a Foo"); let mut bar = TypedBox::::assume_from(bar_erased).expect("it's a Bar");