From b41bb4fe925152b874920c6ea5953934980ed9c8 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 3 Jun 2021 10:08:11 -0700 Subject: [PATCH 01/36] Implement Debug for more things --- aws/rust-runtime/aws-endpoint/src/lib.rs | 2 +- aws/rust-runtime/aws-http/src/user_agent.rs | 2 +- aws/rust-runtime/aws-hyper/src/conn.rs | 19 ++++++++++++++++++- .../aws-sig-auth/src/middleware.rs | 2 +- aws/rust-runtime/aws-sig-auth/src/signer.rs | 8 ++++++++ .../smithy-http-tower/src/map_request.rs | 1 + 6 files changed, 30 insertions(+), 4 deletions(-) diff --git a/aws/rust-runtime/aws-endpoint/src/lib.rs b/aws/rust-runtime/aws-endpoint/src/lib.rs index 8236cb50a0..e2ce7272b0 100644 --- a/aws/rust-runtime/aws-endpoint/src/lib.rs +++ b/aws/rust-runtime/aws-endpoint/src/lib.rs @@ -134,7 +134,7 @@ pub fn set_endpoint_resolver(config: &mut PropertyBag, provider: AwsEndpointReso /// 3. Apply the endpoint to the URI in the request /// 4. Set the `SigningRegion` and `SigningService` in the property bag to drive downstream /// signing middleware. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct AwsEndpointStage; #[derive(Debug)] diff --git a/aws/rust-runtime/aws-http/src/user_agent.rs b/aws/rust-runtime/aws-http/src/user_agent.rs index cc5efddccc..ac811a95bb 100644 --- a/aws/rust-runtime/aws-http/src/user_agent.rs +++ b/aws/rust-runtime/aws-http/src/user_agent.rs @@ -216,7 +216,7 @@ impl Display for ExecEnvMetadata { } #[non_exhaustive] -#[derive(Default, Clone)] +#[derive(Default, Clone, Debug)] pub struct UserAgentStage; impl UserAgentStage { diff --git a/aws/rust-runtime/aws-hyper/src/conn.rs b/aws/rust-runtime/aws-hyper/src/conn.rs index bb5219d695..d32fd4efc1 100644 --- a/aws/rust-runtime/aws-hyper/src/conn.rs +++ b/aws/rust-runtime/aws-hyper/src/conn.rs @@ -8,12 +8,13 @@ use http::Request; use hyper::client::ResponseFuture; use hyper::Response; use smithy_http::body::SdkBody; +use std::fmt; use std::future::Future; use std::pin::Pin; use std::task::{Context, Poll}; use tower::Service; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Standard(Connector); impl Standard { @@ -89,6 +90,22 @@ enum Connector { Dyn(Box), } +impl fmt::Debug for Connector { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + #[cfg(feature = "native-tls")] + Connector::NativeHttps(tls) => { + f.debug_tuple("Connector::NativeHttps").field(tls).finish() + } + #[cfg(feature = "rustls")] + Connector::RustlsHttps(tls) => { + f.debug_tuple("Connector::RustlsHttps").field(tls).finish() + } + Connector::Dyn(_) => f.debug_tuple("Connector::Dyn").finish(), + } + } +} + impl Clone for Box { fn clone(&self) -> Self { self.clone_box() diff --git a/aws/rust-runtime/aws-sig-auth/src/middleware.rs b/aws/rust-runtime/aws-sig-auth/src/middleware.rs index eebbca1075..49effa194f 100644 --- a/aws/rust-runtime/aws-sig-auth/src/middleware.rs +++ b/aws/rust-runtime/aws-sig-auth/src/middleware.rs @@ -26,7 +26,7 @@ use std::time::SystemTime; /// The following fields MAY be present in the property bag: /// - [`SystemTime`](SystemTime): The timestamp to use when signing the request. If this field is not present /// [`SystemTime::now`](SystemTime::now) will be used. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct SigV4SigningStage { signer: SigV4Signer, } diff --git a/aws/rust-runtime/aws-sig-auth/src/signer.rs b/aws/rust-runtime/aws-sig-auth/src/signer.rs index 022fb91050..4a34d3d5e9 100644 --- a/aws/rust-runtime/aws-sig-auth/src/signer.rs +++ b/aws/rust-runtime/aws-sig-auth/src/signer.rs @@ -10,6 +10,7 @@ use aws_types::SigningService; use http::header::HeaderName; use smithy_http::body::SdkBody; use std::error::Error; +use std::fmt; use std::time::SystemTime; #[derive(Eq, PartialEq, Clone, Copy)] @@ -86,6 +87,13 @@ pub struct SigV4Signer { _private: (), } +impl fmt::Debug for SigV4Signer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut formatter = f.debug_struct("SigV4Signer"); + formatter.finish() + } +} + pub type SigningError = Box; impl SigV4Signer { diff --git a/rust-runtime/smithy-http-tower/src/map_request.rs b/rust-runtime/smithy-http-tower/src/map_request.rs index 5f741e3f23..ba63257a0e 100644 --- a/rust-runtime/smithy-http-tower/src/map_request.rs +++ b/rust-runtime/smithy-http-tower/src/map_request.rs @@ -19,6 +19,7 @@ pub struct MapRequestService { mapper: M, } +#[derive(Debug)] pub struct MapRequestLayer { mapper: M, } From ac926ef3ffd992f4b9581d0a20687bc3bf9f9bc3 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 3 Jun 2021 10:08:03 -0700 Subject: [PATCH 02/36] Extract out generic hyper client to smithy-hyper --- aws/rust-runtime/aws-hyper/Cargo.toml | 5 +- aws/rust-runtime/aws-hyper/src/lib.rs | 85 ++--- aws/sdk/build.gradle.kts | 1 + rust-runtime/smithy-hyper/Cargo.toml | 37 ++ rust-runtime/smithy-hyper/LICENSE | 175 +++++++++ rust-runtime/smithy-hyper/src/hyper_impls.rs | 83 +++++ rust-runtime/smithy-hyper/src/lib.rs | 350 ++++++++++++++++++ .../smithy-hyper}/src/retry.rs | 95 +++-- .../smithy-hyper/src/test_connection.rs | 160 ++++++++ 9 files changed, 912 insertions(+), 79 deletions(-) create mode 100644 rust-runtime/smithy-hyper/Cargo.toml create mode 100644 rust-runtime/smithy-hyper/LICENSE create mode 100644 rust-runtime/smithy-hyper/src/hyper_impls.rs create mode 100644 rust-runtime/smithy-hyper/src/lib.rs rename {aws/rust-runtime/aws-hyper => rust-runtime/smithy-hyper}/src/retry.rs (81%) create mode 100644 rust-runtime/smithy-hyper/src/test_connection.rs diff --git a/aws/rust-runtime/aws-hyper/Cargo.toml b/aws/rust-runtime/aws-hyper/Cargo.toml index f982623235..a3c2b01f89 100644 --- a/aws/rust-runtime/aws-hyper/Cargo.toml +++ b/aws/rust-runtime/aws-hyper/Cargo.toml @@ -10,8 +10,8 @@ license = "Apache-2.0" [features] test-util = ["protocol-test-helpers"] default = ["test-util"] -native-tls = ["hyper-tls"] -rustls = ["hyper-rustls"] +native-tls = ["hyper-tls", "smithy-hyper/native-tls"] +rustls = ["hyper-rustls", "smithy-hyper/rustls"] [dependencies] hyper = { version = "0.14.2", features = ["client", "http1", "http2", "tcp", "runtime"] } @@ -28,6 +28,7 @@ http-body = "0.4.0" smithy-http = { path = "../../../rust-runtime/smithy-http" } smithy-types = { path = "../../../rust-runtime/smithy-types" } smithy-http-tower = { path = "../../../rust-runtime/smithy-http-tower" } +smithy-hyper = { path = "../../../rust-runtime/smithy-hyper" } fastrand = "1.4.0" tokio = { version = "1", features = ["time"] } diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index ee2676bb7e..bfc95ed052 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -4,14 +4,12 @@ */ pub mod conn; -mod retry; #[cfg(feature = "test-util")] pub mod test_connection; -pub use retry::RetryConfig; +pub use smithy_hyper::retry::Config as RetryConfig; use crate::conn::Standard; -use crate::retry::RetryHandlerFactory; use aws_endpoint::AwsEndpointStage; use aws_http::user_agent::UserAgentStage; use aws_sig_auth::middleware::SigV4SigningStage; @@ -21,18 +19,43 @@ use smithy_http::operation::Operation; use smithy_http::response::ParseHttpResponse; pub use smithy_http::result::{SdkError, SdkSuccess}; use smithy_http::retry::ClassifyResponse; -use smithy_http_tower::dispatch::DispatchLayer; use smithy_http_tower::map_request::MapRequestLayer; -use smithy_http_tower::parse_response::ParseResponseLayer; use smithy_types::retry::ProvideErrorKind; use std::error::Error; -use std::fmt; -use std::fmt::{Debug, Formatter}; -use tower::{Service, ServiceBuilder, ServiceExt}; +use std::fmt::Debug; +use tower::layer::util::Stack; +use tower::{Service, ServiceBuilder}; type BoxError = Box; pub type StandardClient = Client; +type AwsMiddleware = Stack< + MapRequestLayer, + Stack, MapRequestLayer>, +>; + +#[derive(Debug)] +struct AwsMiddlewareLayer; +impl tower::Layer for AwsMiddlewareLayer { + type Service = >::Service; + + fn layer(&self, inner: S) -> Self::Service { + let signer = MapRequestLayer::for_mapper(SigV4SigningStage::new(SigV4Signer::new())); + let endpoint_resolver = MapRequestLayer::for_mapper(AwsEndpointStage); + let user_agent = MapRequestLayer::for_mapper(UserAgentStage::new()); + // These layers can be considered as occuring in order, that is: + // 1. Resolve an endpoint + // 2. Add a user agent + // 3. Sign + // (4. Dispatch over the wire) + ServiceBuilder::new() + .layer(endpoint_resolver) + .layer(user_agent) + .layer(signer) + .service(inner) + } +} + /// AWS Service Client /// /// Hyper-based AWS Service Client. Most customers will want to construct a client with @@ -48,30 +71,24 @@ pub type StandardClient = Client; /// S::Error: Into + Send + Sync + 'static, /// S::Future: Send + 'static, /// ``` +#[derive(Debug)] pub struct Client { - inner: S, - retry_handler: RetryHandlerFactory, -} - -impl Debug for Client { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let mut formatter = f.debug_struct("Client"); - formatter.field("retry_handler", &self.retry_handler); - formatter.finish() - } + inner: smithy_hyper::Client, } impl Client { /// Construct a new `Client` with a custom connector pub fn new(connector: S) -> Self { Client { - inner: connector, - retry_handler: RetryHandlerFactory::new(RetryConfig::default()), + inner: smithy_hyper::Builder::new() + .connector(connector) + .middleware(AwsMiddlewareLayer) + .build(), } } pub fn with_retry_config(mut self, retry_config: RetryConfig) -> Self { - self.retry_handler.with_config(retry_config); + self.inner.set_retry_config(retry_config); self } } @@ -81,8 +98,10 @@ impl Client { #[cfg(any(feature = "native-tls", feature = "rustls"))] pub fn https() -> StandardClient { Client { - inner: Standard::https(), - retry_handler: RetryHandlerFactory::new(RetryConfig::default()), + inner: smithy_hyper::Builder::new() + .connector(Standard::https()) + .middleware(AwsMiddlewareLayer) + .build(), } } } @@ -119,25 +138,7 @@ where E: Error + ProvideErrorKind, Retry: ClassifyResponse, SdkError>, { - let signer = MapRequestLayer::for_mapper(SigV4SigningStage::new(SigV4Signer::new())); - let endpoint_resolver = MapRequestLayer::for_mapper(AwsEndpointStage); - let user_agent = MapRequestLayer::for_mapper(UserAgentStage::new()); - let inner = self.inner.clone(); - let mut svc = ServiceBuilder::new() - // Create a new request-scoped policy - .retry(self.retry_handler.new_handler()) - .layer(ParseResponseLayer::::new()) - // These layers can be considered as occuring in order, that is: - // 1. Resolve an endpoint - // 2. Add a user agent - // 3. Sign - // 4. Dispatch over the wire - .layer(endpoint_resolver) - .layer(user_agent) - .layer(signer) - .layer(DispatchLayer::new()) - .service(inner); - svc.ready().await?.call(input).await + self.inner.call_raw(input).await } } diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 40db88297e..957ceac1af 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -26,6 +26,7 @@ val runtimeModules = listOf( "smithy-xml", "smithy-http", "smithy-http-tower", + "smithy-hyper", "protocol-test-helpers" ) val awsModules = listOf("aws-auth", "aws-endpoint", "aws-types", "aws-hyper", "aws-sig-auth", "aws-http") diff --git a/rust-runtime/smithy-hyper/Cargo.toml b/rust-runtime/smithy-hyper/Cargo.toml new file mode 100644 index 0000000000..15e2d6d347 --- /dev/null +++ b/rust-runtime/smithy-hyper/Cargo.toml @@ -0,0 +1,37 @@ +[package] +name = "smithy-hyper" +version = "0.1.0" +authors = ["Russell Cohen "] +edition = "2018" +license = "Apache-2.0" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[features] +test-util = ["protocol-test-helpers"] +default = ["test-util"] +native-tls = ["hyper-tls"] +rustls = ["hyper-rustls"] + +[dependencies] +hyper = { version = "0.14.2", features = ["client", "http1", "http2", "tcp", "runtime"] } +tower = { version = "0.4.6", features = ["util", "retry"] } +hyper-tls = { version ="0.5.0", optional = true } +hyper-rustls = { version = "0.22.1", optional = true, features = ["rustls-native-certs"] } +http = "0.2.3" +bytes = "1" +http-body = "0.4.0" +smithy-http = { path = "../smithy-http" } +smithy-types = { path = "../smithy-types" } +smithy-http-tower = { path = "../smithy-http-tower" } +fastrand = "1.4.0" +tokio = { version = "1", features = ["time"] } + +pin-project = "1" +tracing = "0.1.25" + +protocol-test-helpers = { path = "../protocol-test-helpers", optional = true } + +[dev-dependencies] +tokio = { version = "1", features = ["full", "test-util"] } +tower-test = "0.4.0" diff --git a/rust-runtime/smithy-hyper/LICENSE b/rust-runtime/smithy-hyper/LICENSE new file mode 100644 index 0000000000..67db858821 --- /dev/null +++ b/rust-runtime/smithy-hyper/LICENSE @@ -0,0 +1,175 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. diff --git a/rust-runtime/smithy-hyper/src/hyper_impls.rs b/rust-runtime/smithy-hyper/src/hyper_impls.rs new file mode 100644 index 0000000000..2967966322 --- /dev/null +++ b/rust-runtime/smithy-hyper/src/hyper_impls.rs @@ -0,0 +1,83 @@ +use crate::Builder; +use smithy_http::body::SdkBody; +pub use smithy_http::result::{SdkError, SdkSuccess}; +use tower::Service; + +/// Adapter from a [`hyper::Client`] to a connector useable by a [`Client`](crate::Client). +#[derive(Clone, Debug)] +#[non_exhaustive] +pub struct HyperAdapter(hyper::Client); + +impl Service> for HyperAdapter +where + C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, +{ + type Response = http::Response; + type Error = hyper::Error; + type Future = std::pin::Pin< + Box> + Send + 'static>, + >; + + fn poll_ready( + &mut self, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.0.poll_ready(cx) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + let fut = self.0.call(req); + Box::pin(async move { Ok(fut.await?.map(SdkBody::from)) }) + } +} + +impl From> for HyperAdapter { + fn from(hc: hyper::Client) -> Self { + Self(hc) + } +} + +impl Builder { + /// Connect to the service using the provided `hyper` client. + pub fn hyper(self, connector: hyper::Client) -> Builder, M, R> + where + HC: hyper::client::connect::Connect + Clone + Send + Sync + 'static, + { + self.connector(HyperAdapter::from(connector)) + } +} + +#[cfg(feature = "rustls")] +impl Builder { + /// Connect to the service over HTTPS using Rustls. + pub fn rustls( + self, + ) -> Builder>, M, R> + { + let https = hyper_rustls::HttpsConnector::with_native_roots(); + let client = hyper::Client::builder().build::<_, SdkBody>(https); + self.connector(HyperAdapter::from(client)) + } + + /// Connect to the service over HTTPS using Rustls. + /// + /// This is exactly equivalent to [`Builder::rustls`]. If you instead wish to use `native_tls`, + /// use [`Builder::native_tls`]. + pub fn https( + self, + ) -> Builder>, M, R> + { + self.rustls() + } +} +#[cfg(feature = "native-tls")] +impl Builder { + /// Connect to the service over HTTPS using the native TLS library on your platform. + pub fn native_tls( + self, + ) -> Builder>, M, R> { + let https = hyper_tls::HttpsConnector::new(); + let client = hyper::Client::builder().build::<_, SdkBody>(https); + self.connector(HyperAdapter::from(client)) + } +} diff --git a/rust-runtime/smithy-hyper/src/lib.rs b/rust-runtime/smithy-hyper/src/lib.rs new file mode 100644 index 0000000000..a13bbb02a4 --- /dev/null +++ b/rust-runtime/smithy-hyper/src/lib.rs @@ -0,0 +1,350 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ +//! A Hyper-based Smithy service client. +#![warn(missing_debug_implementations, missing_docs, rustdoc::all)] + +pub mod retry; + +#[cfg(feature = "test-util")] +pub mod test_connection; + +mod hyper_impls; +use hyper_impls::HyperAdapter; + +use smithy_http::body::SdkBody; +use smithy_http::operation::Operation; +use smithy_http::response::ParseHttpResponse; +pub use smithy_http::result::{SdkError, SdkSuccess}; +use smithy_http::retry::ClassifyResponse; +use smithy_http_tower::dispatch::DispatchLayer; +use smithy_http_tower::parse_response::ParseResponseLayer; +use smithy_types::retry::ProvideErrorKind; +use std::error::Error; +use std::fmt::Debug; +use tower::{Layer, Service, ServiceBuilder, ServiceExt}; + +type BoxError = Box; + +/// Hyper-based Smithy Service Client. +/// +/// The service client is customizeable in a number of ways (see [`Builder`]), but most customers +/// can stick with the standard constructor provided by [`Client::new`]. It takes only a single +/// argument, which is the middleware that fills out the [`http::Request`] for each higher-level +/// operation so that it can ultimately be sent to the remote host. The middleware is responsible +/// for filling in any request parameters that aren't specified by the Smithy protocol definition, +/// such as those used for routing (like the URL), authentication, and authorization. +/// +/// The middleware takes the form of a [`tower::Layer`] that wraps the actual connection for each +/// request. The [`tower::Service`] that the middleware produces must accept requests of the type +/// [`smithy_http::operation::Request`] and return responses of the type +/// [`http::Response`], most likely by modifying the provided request in place, passing it +/// to the inner service, and then ultimately returning the inner service's response. +#[derive(Debug)] +pub struct Client { + connector: Connector, + middleware: Middleware, + retry_policy: RetryPolicy, +} + +impl Client>, M> { + /// Create a Smithy client that uses HTTPS and the [standard retry policy](retry::Standard). + pub fn new(middleware: M) -> Self { + Builder::new().https().middleware(middleware).build() + } +} + +impl Client { + /// Set the standard retry policy's configuration. + pub fn set_retry_config(&mut self, config: retry::Config) { + self.retry_policy.with_config(config); + } +} + +/// A builder that provides more customization options when constructing a [`Client`]. +/// +/// To start, call [`Builder::new`]. Then, chain the method calls to configure the `Builder`. +/// When configured to your liking, call [`Builder::build`]. The individual methods have additional +/// documentation. +#[derive(Clone, Debug)] +pub struct Builder { + connector: C, + middleware: M, + retry_policy: R, +} + +impl Default for Builder<(), ()> { + fn default() -> Self { + Builder { + connector: (), + middleware: (), + retry_policy: retry::Standard::default(), + } + } +} + +impl Builder<(), ()> { + /// Construct a new, unconfigured builder. + /// + /// This builder cannot yet be used, as it does not specify a [connector](Builder::connector) + /// or [middleware](Builder::middleware). It uses the [standard retry + /// mechanism](retry::Standard). + pub fn new() -> Self { + Self::default() + } +} + +impl Builder { + /// Specify the connector for the eventual client to use. + /// + /// The connector dictates how requests are turned into responses. Normally, this would entail + /// sending the request to some kind of remote server, but in certain settings it's useful to + /// be able to use a custom connector instead, such as to mock the network for tests. + /// + /// If you want to use a custom hyper connector, use [`Builder::hyper`]. + /// + /// If you just want to specify a function from request to response instead, use + /// [`Builder::map_connector`]. + pub fn connector(self, connector: C2) -> Builder { + Builder { + connector, + retry_policy: self.retry_policy, + middleware: self.middleware, + } + } + + /// Specify the middleware for the eventual client ot use. + /// + /// The middleware adjusts requests before they are dispatched to the connector. It is + /// responsible for filling in any request parameters that aren't specified by the Smithy + /// protocol definition, such as those used for routing (like the URL), authentication, and + /// authorization. + /// + /// The middleware takes the form of a [`tower::Layer`] that wraps the actual connection for + /// each request. The [`tower::Service`] that the middleware produces must accept requests of + /// the type [`smithy_http::operation::Request`] and return responses of the type + /// [`http::Response`], most likely by modifying the provided request in place, + /// passing it to the inner service, and then ultimately returning the inner service's + /// response. + /// + /// If your requests are already ready to be sent and need no adjustment, you can use + /// [`tower::layer::util::Identity`] as your middleware. + pub fn middleware(self, middleware: M2) -> Builder { + Builder { + connector: self.connector, + retry_policy: self.retry_policy, + middleware, + } + } + + /// Specify the retry policy for the eventual client to use. + /// + /// By default, the Smithy client uses a standard retry policy that works well in most + /// settings. You can use this method to override that policy with a custom one. A new policy + /// instance will be instantiated for each request using [`retry::NewRequestPolicy`]. Each + /// policy instance must implement [`tower::retry::Policy`]. + /// + /// If you just want to modify the policy _configuration_ for the standard retry policy, use + /// [`Builder::set_retry_config`]. + pub fn retry_policy(self, retry_policy: R2) -> Builder { + Builder { + connector: self.connector, + retry_policy, + middleware: self.middleware, + } + } +} + +impl Builder { + /// Set the standard retry policy's configuration. + pub fn set_retry_config(&mut self, config: retry::Config) { + self.retry_policy.with_config(config); + } +} + +impl Builder { + /// Use a connector that directly maps each request to a response. + /// + /// ```rust + /// use smithy_hyper::Builder; + /// use smithy_http::body::SdkBody; + /// let client = Builder::new() + /// # /* + /// .middleware(..) + /// # */ + /// # .middleware(tower::layer::util::Identity::new()) + /// .map_connector(|req: http::Request| { + /// async move { + /// Ok(http::Response::new(SdkBody::empty())) + /// } + /// }) + /// .build(); + /// # client.check(); + /// ``` + pub fn map_connector(self, map: F) -> Builder, M, R> + where + F: Fn(http::Request) -> FF + Send, + FF: std::future::Future, BoxError>>, + { + self.connector(tower::service_fn(map)) + } +} + +impl Builder { + /// Build a Smithy service [`Client`]. + pub fn build(self) -> Client { + Client { + connector: self.connector, + retry_policy: self.retry_policy, + middleware: self.middleware, + } + } +} + +impl Client +where + C: Service, Response = http::Response> + Send + Clone + 'static, + C::Error: Into + Send + Sync + 'static, + C::Future: Send + 'static, + R: retry::NewRequestPolicy, + M: Layer>, + M::Service: Service< + smithy_http::operation::Request, + Response = http::Response, + Error = smithy_http_tower::SendOperationError, + > + Send + + Clone + + 'static, + >::Future: Send + 'static, +{ + /// Dispatch this request to the network + /// + /// For ergonomics, this does not include the raw response for successful responses. To + /// access the raw response use `call_raw`. + pub async fn call(&self, input: Operation) -> Result> + where + O: ParseHttpResponse> + Send + Sync + Clone + 'static, + E: Error + ProvideErrorKind, + R::Policy: tower::retry::Policy, SdkSuccess, SdkError> + Clone, + Retry: ClassifyResponse, SdkError>, + { + self.call_raw(input).await.map(|res| res.parsed) + } + + /// Dispatch this request to the network + /// + /// The returned result contains the raw HTTP response which can be useful for debugging or + /// implementing unsupported features. + pub async fn call_raw( + &self, + input: Operation, + ) -> Result, SdkError> + where + O: ParseHttpResponse> + Send + Sync + Clone + 'static, + E: Error + ProvideErrorKind, + R::Policy: tower::retry::Policy, SdkSuccess, SdkError> + Clone, + Retry: ClassifyResponse, SdkError>, + { + let connector = self.connector.clone(); + let mut svc = ServiceBuilder::new() + // Create a new request-scoped policy + .retry(self.retry_policy.new_request_policy()) + .layer(ParseResponseLayer::::new()) + // These layers can be considered as occuring in order. That is, first invoke the + // customer-provided middleware, then dispatch dispatch over the wire. + .layer(&self.middleware) + .layer(DispatchLayer::new()) + .check_service() + .service(connector); + svc.ready().await?.call(input).await + } + + /// Statically check the validity of a `Client` without a request to send. + /// + /// This will make sure that all the bounds hold that would be required by `call` and + /// `call_raw` (modulo those that relate to the specific `Operation` type). Comes in handy to + /// ensure (statically) that all the various constructors actually produce "useful" types. + #[doc(hidden)] + pub fn check(&self) + where + R::Policy: tower::retry::Policy< + static_tests::ValidTestOperation, + SdkSuccess<()>, + SdkError, + > + Clone, + { + let _ = |o: static_tests::ValidTestOperation| { + let _ = self.call_raw(o); + }; + } +} + +/// This module provides types useful for static tests. +/// +/// These are only used to write the bounds in [`Client::check`]. Customers will not need them. +/// But the module and its types must be public so that we can call `check` from doc-tests. +#[doc(hidden)] +#[allow(missing_docs, missing_debug_implementations)] +pub mod static_tests { + use super::*; + + #[derive(Debug)] + #[non_exhaustive] + pub struct TestOperationError; + impl std::fmt::Display for TestOperationError { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + unreachable!("only used for static tests") + } + } + impl Error for TestOperationError {} + impl ProvideErrorKind for TestOperationError { + fn retryable_error_kind(&self) -> Option { + unreachable!("only used for static tests") + } + + fn code(&self) -> Option<&str> { + unreachable!("only used for static tests") + } + } + #[derive(Clone)] + #[non_exhaustive] + pub struct TestOperation; + impl ParseHttpResponse for TestOperation { + type Output = Result<(), TestOperationError>; + + fn parse_unloaded(&self, _: &mut http::Response) -> Option { + unreachable!("only used for static tests") + } + + fn parse_loaded(&self, _response: &http::Response) -> Self::Output { + unreachable!("only used for static tests") + } + } + pub type ValidTestOperation = Operation; + + // Statically check that a standard retry can actually be used to build a Client. + #[allow(dead_code)] + #[cfg(test)] + fn sanity_retry() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .check(); + } + + // Statically check that a hyper client can actually be used to build a Client. + #[allow(dead_code)] + #[cfg(test)] + fn sanity_hyper(hc: hyper::Client) + where + C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, + { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .hyper(hc) + .build() + .check(); + } +} diff --git a/aws/rust-runtime/aws-hyper/src/retry.rs b/rust-runtime/smithy-hyper/src/retry.rs similarity index 81% rename from aws/rust-runtime/aws-hyper/src/retry.rs rename to rust-runtime/smithy-hyper/src/retry.rs index 08d003dff6..f982225215 100644 --- a/aws/rust-runtime/aws-hyper/src/retry.rs +++ b/rust-runtime/smithy-hyper/src/retry.rs @@ -10,12 +10,11 @@ //! implementation is intended to be _correct_ but not especially long lasting. //! //! Components: -//! - [`RetryHandlerFactory`](crate::retry::RetryHandlerFactory): Top level manager, intended -//! to be associated with a [`Client`](crate::Client). Its sole purpose in life is to create a RetryHandler -//! for individual requests. -//! - [`RetryHandler`](crate::retry::RetryHandler): A request-scoped retry policy, -//! backed by request-local state and shared state contained within [`RetryHandlerFactory`](crate::retry::RetryHandlerFactory) -//! - [`RetryConfig`](crate::retry::RetryConfig): Static configuration (max retries, max backoff etc.) +//! - [`Standard`]: Top level manager, intended to be associated with a [`Client`](crate::Client). +//! Its sole purpose in life is to create a [`RetryHandler`] for individual requests. +//! - [`RetryHandler`]: A request-scoped retry policy, backed by request-local state and shared +//! state contained within [`Standard`]. +//! - [`Config`]: Static configuration (max retries, max backoff etc.) use crate::{SdkError, SdkSuccess}; use smithy_http::operation; @@ -28,13 +27,26 @@ use std::sync::{Arc, Mutex}; use std::time::Duration; use tracing::Instrument; +/// A policy instantiator. +/// +/// Implementors are essentially "policy factories" that can produce a new instance of a retry +/// policy mechanism for each request, which allows both shared global state _and_ per-request +/// local state. +pub trait NewRequestPolicy { + /// The type of the per-request policy mechanism. + type Policy; + + /// Create a new policy mechanism instance. + fn new_request_policy(&self) -> Self::Policy; +} + /// Retry Policy Configuration /// -/// Without specific use cases, users should generally rely on the default values set by `[RetryConfig::default]`(RetryConfig::default).` +/// Without specific use cases, users should generally rely on the default values set by `[Config::default]`(Config::default).` /// /// Currently these fields are private and no setters provided. As needed, this configuration will become user-modifiable in the future.. #[derive(Clone, Debug)] -pub struct RetryConfig { +pub struct Config { initial_retry_tokens: usize, retry_cost: usize, no_retry_increment: usize, @@ -44,14 +56,14 @@ pub struct RetryConfig { base: fn() -> f64, } -impl RetryConfig { +impl Config { /// Override `b` in the exponential backoff computation /// /// By default, `base` is a randomly generated value between 0 and 1. In tests, it can /// be helpful to override this: /// ```rust - /// use aws_hyper::RetryConfig; - /// let conf = RetryConfig::default().with_base(||1_f64); + /// use smithy_hyper::retry::Config; + /// let conf = Config::default().with_base(||1_f64); /// ``` pub fn with_base(mut self, base: fn() -> f64) -> Self { self.base = base; @@ -59,7 +71,7 @@ impl RetryConfig { } } -impl Default for RetryConfig { +impl Default for Config { fn default() -> Self { Self { initial_retry_tokens: INITIAL_RETRY_TOKENS, @@ -81,31 +93,38 @@ const RETRY_COST: usize = 5; /// Manage retries for a service /// /// An implementation of the `standard` AWS retry strategy as specified in the SEP. A `Strategy` is scoped to a client. -/// For an individual request, call [`RetryHandlerFactory::new_handler()`](RetryHandlerFactory::new_handler) +/// For an individual request, call [`Standard::new_request_policy()`](Standard::new_request_policy) /// /// In the future, adding support for the adaptive retry strategy will be added by adding a `TokenBucket` to /// `CrossRequestRetryState` -/// Its main functionality is via `new_handler` which creates a `RetryHandler` to manage the retry for +/// Its main functionality is via `new_request_policy` which creates a `RetryHandler` to manage the retry for /// an individual request. #[derive(Debug)] -pub struct RetryHandlerFactory { - config: RetryConfig, +pub struct Standard { + config: Config, shared_state: CrossRequestRetryState, } -impl RetryHandlerFactory { - pub fn new(config: RetryConfig) -> Self { +impl Standard { + /// Construct a new standard retry policy from the given policy configuration. + pub fn new(config: Config) -> Self { Self { shared_state: CrossRequestRetryState::new(config.initial_retry_tokens), config, } } - pub fn with_config(&mut self, config: RetryConfig) { + /// Set the configuration for this retry policy. + pub fn with_config(&mut self, config: Config) -> &mut Self { self.config = config; + self } +} + +impl NewRequestPolicy for Standard { + type Policy = RetryHandler; - pub(crate) fn new_handler(&self) -> RetryHandler { + fn new_request_policy(&self) -> Self::Policy { RetryHandler { local: RequestLocalRetryState::new(), shared: self.shared_state.clone(), @@ -114,7 +133,13 @@ impl RetryHandlerFactory { } } -#[derive(Default, Clone)] +impl Default for Standard { + fn default() -> Self { + Self::new(Config::default()) + } +} + +#[derive(Default, Clone, Debug)] struct RequestLocalRetryState { attempts: u32, last_quota_usage: Option, @@ -148,7 +173,7 @@ impl CrossRequestRetryState { } } - fn quota_release(&self, value: Option, config: &RetryConfig) { + fn quota_release(&self, value: Option, config: &Config) { let mut quota = self.quota_available.lock().unwrap(); *quota += value.unwrap_or(config.no_retry_increment); } @@ -157,7 +182,7 @@ impl CrossRequestRetryState { /// /// If quota is available, the amount of quota consumed is returned /// If no quota is available, `None` is returned. - fn quota_acquire(&self, err: &ErrorKind, config: &RetryConfig) -> Option { + fn quota_acquire(&self, err: &ErrorKind, config: &Config) -> Option { let mut quota = self.quota_available.lock().unwrap(); let retry_cost = if err == &ErrorKind::TransientError { config.timeout_retry_cost @@ -178,11 +203,11 @@ impl CrossRequestRetryState { /// Implement retries for an individual request. /// It is intended to be used as a [Tower Retry Policy](tower::retry::Policy) for use in tower-based /// middleware stacks. -#[derive(Clone)] -pub(crate) struct RetryHandler { +#[derive(Clone, Debug)] +pub struct RetryHandler { local: RequestLocalRetryState, shared: CrossRequestRetryState, - config: RetryConfig, + config: Config, } #[cfg(test)] @@ -197,7 +222,7 @@ impl RetryHandler { /// /// If a retry is specified, this function returns `(next, backoff_duration)` /// If no retry is specified, this function returns None - pub fn attempt_retry(&self, retry_kind: Result<(), ErrorKind>) -> Option<(Self, Duration)> { + fn attempt_retry(&self, retry_kind: Result<(), ErrorKind>) -> Option<(Self, Duration)> { let quota_used = match retry_kind { Ok(_) => { self.shared @@ -277,14 +302,14 @@ fn check_send_sync(t: T) -> T { #[cfg(test)] mod test { - use crate::retry::{RetryConfig, RetryHandler, RetryHandlerFactory}; + use crate::retry::{Config, NewRequestPolicy, RetryHandler, Standard}; use smithy_types::retry::ErrorKind; use std::time::Duration; fn assert_send_sync() {} - fn test_config() -> RetryConfig { - RetryConfig::default().with_base(|| 1_f64) + fn test_config() -> Config { + Config::default().with_base(|| 1_f64) } #[test] @@ -294,7 +319,7 @@ mod test { #[test] fn eventual_success() { - let policy = RetryHandlerFactory::new(test_config()).new_handler(); + let policy = Standard::new(test_config()).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) .expect("should retry"); @@ -314,7 +339,7 @@ mod test { #[test] fn no_more_attempts() { - let policy = RetryHandlerFactory::new(test_config()).new_handler(); + let policy = Standard::new(test_config()).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) .expect("should retry"); @@ -336,7 +361,7 @@ mod test { fn no_quota() { let mut conf = test_config(); conf.initial_retry_tokens = 5; - let policy = RetryHandlerFactory::new(conf).new_handler(); + let policy = Standard::new(conf).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) .expect("should retry"); @@ -351,7 +376,7 @@ mod test { fn backoff_timing() { let mut conf = test_config(); conf.max_retries = 5; - let policy = RetryHandlerFactory::new(conf).new_handler(); + let policy = Standard::new(conf).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) .expect("should retry"); @@ -386,7 +411,7 @@ mod test { let mut conf = test_config(); conf.max_retries = 5; conf.max_backoff = Duration::from_secs(3); - let policy = RetryHandlerFactory::new(conf).new_handler(); + let policy = Standard::new(conf).new_request_policy(); let (policy, dur) = policy .attempt_retry(Err(ErrorKind::ServerError)) .expect("should retry"); diff --git a/rust-runtime/smithy-hyper/src/test_connection.rs b/rust-runtime/smithy-hyper/src/test_connection.rs new file mode 100644 index 0000000000..93fa60c1d9 --- /dev/null +++ b/rust-runtime/smithy-hyper/src/test_connection.rs @@ -0,0 +1,160 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ +//! Module with client connectors usefule for testing. + +// TODO +#![allow(missing_docs)] + +use http::header::{HeaderName, CONTENT_TYPE}; +use http::Request; +use protocol_test_helpers::{assert_ok, validate_body, MediaType}; +use smithy_http::body::SdkBody; +use std::future::Ready; +use std::ops::Deref; +use std::sync::{Arc, Mutex}; +use std::task::{Context, Poll}; +use tower::BoxError; + +type ConnectVec = Vec<(http::Request, http::Response)>; + +#[derive(Debug)] +pub struct ValidateRequest { + pub expected: http::Request, + pub actual: http::Request, +} + +impl ValidateRequest { + pub fn assert_matches(&self, ignore_headers: Vec) { + let (actual, expected) = (&self.actual, &self.expected); + for (name, value) in expected.headers() { + if !ignore_headers.contains(name) { + let actual_header = actual + .headers() + .get(name) + .unwrap_or_else(|| panic!("Header {:?} missing", name)); + assert_eq!(actual_header, value, "Header mismatch for {:?}", name); + } + } + let actual_str = std::str::from_utf8(actual.body().bytes().unwrap_or(&[])); + let expected_str = std::str::from_utf8(expected.body().bytes().unwrap_or(&[])); + let media_type = if actual + .headers() + .get(CONTENT_TYPE) + .map(|v| v.to_str().unwrap().contains("json")) + .unwrap_or(false) + { + MediaType::Json + } else { + MediaType::Other("unknown".to_string()) + }; + match (actual_str, expected_str) { + (Ok(actual), Ok(expected)) => assert_ok(validate_body(actual, expected, media_type)), + _ => assert_eq!(actual.body().bytes(), expected.body().bytes()), + }; + assert_eq!(actual.uri(), expected.uri()); + } +} + +/// TestConnection for use with a [`smithy_hyper::Client`](crate::Client) +/// +/// A basic test connection. It will: +/// - Response to requests with a preloaded series of responses +/// - Record requests for future examination +/// +/// The generic parameter `B` is the type of the response body. +/// For more complex use cases, see [Tower Test](https://docs.rs/tower-test/0.4.0/tower_test/) +/// Usage example: +/// ```rust +/// use smithy_hyper::test_connection::TestConnection; +/// use smithy_http::body::SdkBody; +/// let events = vec![( +/// http::Request::new(SdkBody::from("request body")), +/// http::Response::builder() +/// .status(200) +/// .body("response body") +/// .unwrap(), +/// )]; +/// let conn = TestConnection::new(events); +/// let client = smithy_hyper::Client::from(conn); +/// ``` +#[derive(Debug)] +pub struct TestConnection { + data: Arc>>, + requests: Arc>>, +} + +// Need a clone impl that ignores `B` +impl Clone for TestConnection { + fn clone(&self) -> Self { + TestConnection { + data: self.data.clone(), + requests: self.requests.clone(), + } + } +} + +impl TestConnection { + pub fn new(mut data: ConnectVec) -> Self { + data.reverse(); + TestConnection { + data: Arc::new(Mutex::new(data)), + requests: Default::default(), + } + } + + pub fn requests(&self) -> impl Deref> + '_ { + self.requests.lock().unwrap() + } +} + +impl> tower::Service> for TestConnection { + type Response = http::Response; + type Error = BoxError; + type Future = Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, actual: Request) -> Self::Future { + // todo: validate request + if let Some((expected, resp)) = self.data.lock().unwrap().pop() { + self.requests + .lock() + .unwrap() + .push(ValidateRequest { expected, actual }); + std::future::ready(Ok(resp.map(|body| SdkBody::from(body.into())))) + } else { + std::future::ready(Err("No more data".into())) + } + } +} + +impl From> for crate::Client, tower::layer::util::Identity> +where + B: Into + Send + 'static, +{ + fn from(tc: TestConnection) -> Self { + crate::Builder::new() + .middleware(tower::layer::util::Identity::new()) + .connector(tc) + .build() + } +} + +#[cfg(test)] +mod tests { + use crate::test_connection::TestConnection; + use crate::Client; + + fn is_send_sync(_: T) {} + + #[test] + fn construct_test_client() { + let test_conn = TestConnection::::new(vec![]); + let client: Client<_, _, _> = test_conn.into(); + is_send_sync(client); + } +} From c4684588436e86b69e8fda6359ff3267b573bcee Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 3 Jun 2021 14:43:36 -0700 Subject: [PATCH 03/36] Add generic fluent client generation --- aws/sdk/build.gradle.kts | 3 + .../rust/codegen/rustlang/CargoDependency.kt | 3 + .../rust/codegen/smithy/RuntimeTypes.kt | 4 +- .../rust/codegen/smithy/RustSettings.kt | 5 +- .../smithy/customize/RustCodegenDecorator.kt | 3 +- .../generators/FluentClientDecorator.kt | 264 ++++++++++++++++++ .../generators/HttpProtocolGenerator.kt | 23 +- 7 files changed, 297 insertions(+), 8 deletions(-) create mode 100644 codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 957ceac1af..9a87331327 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -104,6 +104,9 @@ fun generateSmithyBuild(tests: List): String { "runtimeConfig": { "relativePath": "../" }, + "codegen": { + "includeFluentClient": false + }, "service": "${it.service}", "module": "aws-sdk-${it.module}", "moduleVersion": "0.0.6-alpha", diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt index 1e190610f1..2c7b8ac981 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt @@ -182,9 +182,12 @@ data class CargoDependency( companion object { val FastRand = CargoDependency("fastrand", CratesIo("1")) val Http: CargoDependency = CargoDependency("http", CratesIo("0.2")) + val Tower: CargoDependency = CargoDependency("tower", CratesIo("0.4"), optional = true) fun SmithyTypes(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("types") fun SmithyHttp(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("http") + fun SmithyHttpTower(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("http-tower") + fun SmithyHyper(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("hyper", true) fun ProtocolTestHelpers(runtimeConfig: RuntimeConfig) = CargoDependency( "protocol-test-helpers", runtimeConfig.runtimeCrateLocation.crateLocation(), scope = DependencyScope.Dev diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt index 2eba3ecbbc..5f262d3562 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt @@ -52,8 +52,8 @@ data class RuntimeConfig( } } - fun runtimeCrate(runtimeCrateName: String): CargoDependency = - CargoDependency("$cratePrefix-$runtimeCrateName", runtimeCrateLocation.crateLocation()) + fun runtimeCrate(runtimeCrateName: String, optional: Boolean = false): CargoDependency = + CargoDependency("$cratePrefix-$runtimeCrateName", runtimeCrateLocation.crateLocation(), optional = optional) } data class RuntimeType(val name: String?, val dependency: RustDependency?, val namespace: String) { diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt index ad1429bb6e..e2121ca7c9 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustSettings.kt @@ -28,12 +28,13 @@ private const val RUNTIME_CONFIG = "runtimeConfig" private const val CODEGEN_SETTINGS = "codegen" private const val LICENSE = "license" -data class CodegenConfig(val renameExceptions: Boolean = true) { +data class CodegenConfig(val renameExceptions: Boolean = true, val includeFluentClient: Boolean = true) { companion object { fun fromNode(node: Optional): CodegenConfig { return if (node.isPresent) { CodegenConfig( - node.get().getBooleanMemberOrDefault("renameErrors", true) + node.get().getBooleanMemberOrDefault("renameErrors", true), + node.get().getBooleanMemberOrDefault("includeFluentClient", true) ) } else { CodegenConfig() diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customize/RustCodegenDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customize/RustCodegenDecorator.kt index c649afdd91..57b91ea067 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customize/RustCodegenDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customize/RustCodegenDecorator.kt @@ -10,6 +10,7 @@ import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.ShapeId import software.amazon.smithy.rust.codegen.smithy.RustCrate import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider +import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientDecorator import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig import software.amazon.smithy.rust.codegen.smithy.generators.config.ConfigCustomization @@ -125,7 +126,7 @@ open class CombinedCodegenDecorator(decorators: List) : Ru .onEach { logger.info("Adding Codegen Decorator: ${it.javaClass.name}") }.toList() - return CombinedCodegenDecorator(decorators + RequiredCustomizations()) + return CombinedCodegenDecorator(decorators + RequiredCustomizations() + FluentClientDecorator()) } } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt new file mode 100644 index 0000000000..fde46b240f --- /dev/null +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -0,0 +1,264 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.rust.codegen.smithy.generators + +import software.amazon.smithy.model.knowledge.TopDownIndex +import software.amazon.smithy.model.shapes.MemberShape +import software.amazon.smithy.rust.codegen.rustlang.Attribute +import software.amazon.smithy.rust.codegen.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.rustlang.Feature +import software.amazon.smithy.rust.codegen.rustlang.RustMetadata +import software.amazon.smithy.rust.codegen.rustlang.RustModule +import software.amazon.smithy.rust.codegen.rustlang.RustType +import software.amazon.smithy.rust.codegen.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.rustlang.asType +import software.amazon.smithy.rust.codegen.rustlang.contains +import software.amazon.smithy.rust.codegen.rustlang.documentShape +import software.amazon.smithy.rust.codegen.rustlang.render +import software.amazon.smithy.rust.codegen.rustlang.rust +import software.amazon.smithy.rust.codegen.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.rustlang.stripOuter +import software.amazon.smithy.rust.codegen.rustlang.writable +import software.amazon.smithy.rust.codegen.smithy.RustCrate +import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator +import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization +import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection +import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig +import software.amazon.smithy.rust.codegen.smithy.generators.builderSymbol +import software.amazon.smithy.rust.codegen.smithy.generators.error.errorSymbol +import software.amazon.smithy.rust.codegen.smithy.generators.setterName +import software.amazon.smithy.rust.codegen.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.smithy.rustType +import software.amazon.smithy.rust.codegen.util.inputShape +import software.amazon.smithy.rust.codegen.util.outputShape +import software.amazon.smithy.rust.codegen.util.toSnakeCase + +class FluentClientDecorator : RustCodegenDecorator { + override val name: String = "FluentClient" + override val order: Byte = 0 + + private fun applies(protocolConfig: ProtocolConfig): Boolean = protocolConfig.symbolProvider.config().codegenConfig.includeFluentClient + + override fun extras(protocolConfig: ProtocolConfig, rustCrate: RustCrate) { + if (!applies(protocolConfig)) { + return; + } + + val module = RustMetadata(additionalAttributes = listOf(Attribute.Cfg.feature("client")), public = true) + rustCrate.withModule(RustModule("client", module)) { writer -> + FluentClientGenerator(protocolConfig).render(writer) + } + val smithyHyper = CargoDependency.SmithyHyper(protocolConfig.runtimeConfig) + rustCrate.addFeature(Feature("client", true, listOf(smithyHyper.name, CargoDependency.Tower.name))) + rustCrate.addFeature(Feature("rustls", default = true, listOf("smithy-hyper/rustls"))) + rustCrate.addFeature(Feature("native-tls", default = false, listOf("smithy-hyper/native-tls"))) + } + + override fun libRsCustomizations( + protocolConfig: ProtocolConfig, + baseCustomizations: List + ): List { + if (!applies(protocolConfig)) { + return baseCustomizations; + } + + return baseCustomizations + object : LibRsCustomization() { + override fun section(section: LibRsSection) = when (section) { + is LibRsSection.Body -> writable { + Attribute.Cfg.feature("client").render(this) + rust("pub use client::Client;") + } + else -> emptySection + } + } + } +} + +class FluentClientGenerator(protocolConfig: ProtocolConfig) { + private val serviceShape = protocolConfig.serviceShape + private val operations = + TopDownIndex.of(protocolConfig.model).getContainedOperations(serviceShape).sortedBy { it.id } + private val symbolProvider = protocolConfig.symbolProvider + private val model = protocolConfig.model + private val hyperDep = CargoDependency.SmithyHyper(protocolConfig.runtimeConfig).copy(optional = true) + private val runtimeConfig = protocolConfig.runtimeConfig + + fun render(writer: RustWriter) { + writer.rustTemplate( + """ + ##[derive(std::fmt::Debug)] + pub(crate) struct Handle { + client: #{smithy_hyper}::Client, + conf: crate::Config, + } + + ##[derive(Clone, std::fmt::Debug)] + pub struct Client { + handle: std::sync::Arc> + } + + impl From<#{smithy_hyper}::Client> for Client { + fn from(client: #{smithy_hyper}::Client) -> Self { + Self::with_config(client, crate::Config::builder().build()) + } + } + """, + "smithy_hyper" to hyperDep.asType() + ) + writer.rustBlock("impl Client") { + rustTemplate( + """ + pub fn with_config(client: #{smithy_hyper}::Client, conf: crate::Config) -> Self { + Self { + handle: std::sync::Arc::new(Handle { + client, + conf, + }) + } + } + + pub fn conf(&self) -> &crate::Config { + &self.handle.conf + } + + """, + "smithy_hyper" to hyperDep.asType() + ) + operations.forEach { operation -> + val name = symbolProvider.toSymbol(operation).name + rust( + """ + pub fn ${name.toSnakeCase()}(&self) -> fluent_builders::$name { + fluent_builders::$name::new(self.handle.clone()) + }""" + ) + } + } + writer.withModule("fluent_builders") { + operations.forEach { operation -> + val name = symbolProvider.toSymbol(operation).name + val input = operation.inputShape(model) + val members: List = input.allMembers.values.toList() + + rust( + """ + ##[derive(std::fmt::Debug)] + pub struct $name { + handle: std::sync::Arc>, + inner: #T + }""", + input.builderSymbol(symbolProvider) + ) + + rustBlock("impl $name") { + rustTemplate( + """ + pub(crate) fn new(handle: std::sync::Arc>) -> Self { + Self { handle, inner: Default::default() } + } + + pub async fn send(self) -> Result<#{ok}, #{sdk_err}<#{operation_err}>> where + C: #{tower_service}<#{http}::Request<#{sdk_body}>, Response = #{http}::Response<#{sdk_body}>> + Send + Clone + 'static, + C::Error: Into> + Send + Sync + 'static, + C::Future: Send + 'static, + R: #{nrpolicy}, + M: #{tower_layer}<#{http_tower}::dispatch::DispatchService>, + M::Service: #{tower_service}< + #{opreq}, + Response = #{http}::Response<#{sdk_body}>, + Error = #{http_tower}::SendOperationError, + > + Send + + Clone + + 'static, + >::Future: Send + 'static, + R::Policy: #{tower_retry_Policy}<#{opmod}OperationAlias, #{sdk_ok}<#{ok}>, #{sdk_err}<#{operation_err}>> + Clone, + { + let input = self.inner.build().map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; + let op = input.make_operation(&self.handle.conf) + .map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; + self.handle.client.call(op).await + } + """, + "ok" to symbolProvider.toSymbol(operation.outputShape(model)), + "opmod" to symbolProvider.toSymbol(operation.inputShape(model)), + "operation_err" to operation.errorSymbol(symbolProvider), + "sdk_ok" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkSuccess"), + "sdk_err" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkError"), + "sdk_body" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "body::SdkBody"), + "opreq" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "operation::Request"), + "op_t" to RuntimeType.operation(runtimeConfig), + "http_tower" to CargoDependency.SmithyHttpTower(runtimeConfig).asType(), + "nrpolicy" to CargoDependency.SmithyHyper(runtimeConfig).asType().copy(name = "retry::NewRequestPolicy"), + "tower_service" to CargoDependency.Tower.asType().copy(name = "Service"), + "tower_layer" to CargoDependency.Tower.asType().copy(name = "layer::Layer"), + "tower_retry_policy" to CargoDependency.Tower.asType().copy(name = "retry::Policy"), + "http" to CargoDependency.Http.asType(), + ) + members.forEach { member -> + val memberName = symbolProvider.toMemberName(member) + // All fields in the builder are optional + val memberSymbol = symbolProvider.toSymbol(member) + val outerType = memberSymbol.rustType() + val coreType = outerType.stripOuter() + when (coreType) { + is RustType.Vec -> renderVecHelper(member, memberName, coreType) + is RustType.HashMap -> renderMapHelper(member, memberName, coreType) + else -> { + val signature = when (coreType) { + is RustType.String, + is RustType.Box -> "(mut self, inp: impl Into<${coreType.render(true)}>) -> Self" + else -> "(mut self, inp: ${coreType.render(true)}) -> Self" + } + documentShape(member, model) + rustBlock("pub fn $memberName$signature") { + write("self.inner = self.inner.$memberName(inp);") + write("self") + } + } + } + // pure setter + rustBlock("pub fn ${member.setterName()}(mut self, inp: ${outerType.render(true)}) -> Self") { + rust( + """ + self.inner = self.inner.${member.setterName()}(inp); + self + """ + ) + } + } + } + } + } + } + + private fun RustWriter.renderMapHelper(member: MemberShape, memberName: String, coreType: RustType.HashMap) { + documentShape(member, model) + val k = coreType.key + val v = coreType.member + + rustBlock("pub fn $memberName(mut self, k: impl Into<${k.render()}>, v: impl Into<${v.render()}>) -> Self") { + rust( + """ + self.inner = self.inner.$memberName(k, v); + self + """ + ) + } + } + + private fun RustWriter.renderVecHelper(member: MemberShape, memberName: String, coreType: RustType.Vec) { + documentShape(member, model) + rustBlock("pub fn $memberName(mut self, inp: impl Into<${coreType.member.render(true)}>) -> Self") { + rust( + """ + self.inner = self.inner.$memberName(inp); + self + """ + ) + } + } +} diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt index ce4a3d91c2..82afb71905 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt @@ -72,6 +72,12 @@ abstract class HttpProtocolGenerator( val builderGenerator = BuilderGenerator(model, symbolProvider, operationShape.inputShape(model)) builderGenerator.render(inputWriter) + // TODO: One day, it should be possible for callers to invoke + // buildOperationType directly to get the type rather than depending on + // this alias. + val baseReturnType = buildOperationType(inputWriter, operationShape, customizations) + inputWriter.rust("pub type ${inputShape.id.name}OperationAlias = $baseReturnType;") + // impl OperationInputShape { ... } inputWriter.implBlock(inputShape, symbolProvider) { buildOperation(this, operationShape, customizations, sdkId) @@ -130,6 +136,19 @@ abstract class HttpProtocolGenerator( abstract fun RustWriter.body(self: String, operationShape: OperationShape): BodyMetadata + private fun buildOperationType( + writer: RustWriter, + shape: OperationShape, + features: List, + ): String { + val runtimeConfig = protocolConfig.runtimeConfig + val outputSymbol = symbolProvider.toSymbol(shape) + val operationT = RuntimeType.operation(runtimeConfig) + val retryType = features.mapNotNull { it.retryType() }.firstOrNull()?.let { writer.format(it) } ?: "()" + + return with(writer) { "${format(operationT)}<${format(outputSymbol)}, $retryType>" }; + } + private fun buildOperation( implBlockWriter: RustWriter, shape: OperationShape, @@ -138,12 +157,10 @@ abstract class HttpProtocolGenerator( ) { val runtimeConfig = protocolConfig.runtimeConfig val outputSymbol = symbolProvider.toSymbol(shape) - val operationT = RuntimeType.operation(runtimeConfig) val operationModule = RuntimeType.operationModule(runtimeConfig) val sdkBody = RuntimeType.sdkBody(runtimeConfig) - val retryType = features.mapNotNull { it.retryType() }.firstOrNull()?.let { implBlockWriter.format(it) } ?: "()" - val baseReturnType = with(implBlockWriter) { "${format(operationT)}<${format(outputSymbol)}, $retryType>" } + val baseReturnType = buildOperationType(implBlockWriter, shape, features) val returnType = "Result<$baseReturnType, ${implBlockWriter.format(runtimeConfig.operationBuildError())}>" implBlockWriter.docs("Consumes the builder and constructs an Operation<#D>", outputSymbol) From c85d0462d59a0bc7c488417304ef18538e1e5c1d Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 3 Jun 2021 17:23:27 -0700 Subject: [PATCH 04/36] Make the bounds nicer --- .../generators/FluentClientDecorator.kt | 76 ++++--- .../generators/HttpProtocolGenerator.kt | 33 ++- rust-runtime/smithy-hyper/src/hyper_impls.rs | 8 + rust-runtime/smithy-hyper/src/lib.rs | 192 +++++++++++++++--- 4 files changed, 232 insertions(+), 77 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index fde46b240f..4a1d6b5f12 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -20,6 +20,7 @@ import software.amazon.smithy.rust.codegen.rustlang.documentShape import software.amazon.smithy.rust.codegen.rustlang.render import software.amazon.smithy.rust.codegen.rustlang.rust import software.amazon.smithy.rust.codegen.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.rustlang.stripOuter import software.amazon.smithy.rust.codegen.rustlang.writable @@ -53,7 +54,7 @@ class FluentClientDecorator : RustCodegenDecorator { FluentClientGenerator(protocolConfig).render(writer) } val smithyHyper = CargoDependency.SmithyHyper(protocolConfig.runtimeConfig) - rustCrate.addFeature(Feature("client", true, listOf(smithyHyper.name, CargoDependency.Tower.name))) + rustCrate.addFeature(Feature("client", true, listOf(smithyHyper.name))) rustCrate.addFeature(Feature("rustls", default = true, listOf("smithy-hyper/rustls"))) rustCrate.addFeature(Feature("native-tls", default = false, listOf("smithy-hyper/native-tls"))) } @@ -92,27 +93,23 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { """ ##[derive(std::fmt::Debug)] pub(crate) struct Handle { - client: #{smithy_hyper}::Client, + client: #{hyper}::Client, conf: crate::Config, } ##[derive(Clone, std::fmt::Debug)] - pub struct Client { + pub struct Client { handle: std::sync::Arc> } - impl From<#{smithy_hyper}::Client> for Client { - fn from(client: #{smithy_hyper}::Client) -> Self { + impl From<#{hyper}::Client> for Client { + fn from(client: #{hyper}::Client) -> Self { Self::with_config(client, crate::Config::builder().build()) } } - """, - "smithy_hyper" to hyperDep.asType() - ) - writer.rustBlock("impl Client") { - rustTemplate( - """ - pub fn with_config(client: #{smithy_hyper}::Client, conf: crate::Config) -> Self { + + impl Client { + pub fn with_config(client: #{hyper}::Client, conf: crate::Config) -> Self { Self { handle: std::sync::Arc::new(Handle { client, @@ -124,10 +121,20 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { pub fn conf(&self) -> &crate::Config { &self.handle.conf } - + } + """, + "hyper" to hyperDep.asType() + ) + writer.rustBlockTemplate( + """ + impl Client + where + C: #{hyper}::bounds::SmithyConnector, + M: #{hyper}::bounds::SmithyMiddleware, + R: #{hyper}::retry::NewRequestPolicy, """, - "smithy_hyper" to hyperDep.asType() - ) + "hyper" to hyperDep.asType(), + ) { operations.forEach { operation -> val name = symbolProvider.toSymbol(operation).name rust( @@ -154,7 +161,16 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { input.builderSymbol(symbolProvider) ) - rustBlock("impl $name") { + rustBlockTemplate( + """ + impl $name + where + C: #{hyper}::bounds::SmithyConnector, + M: #{hyper}::bounds::SmithyMiddleware, + R: #{hyper}::retry::NewRequestPolicy, + """, + "hyper" to CargoDependency.SmithyHyper(runtimeConfig).asType(), + ) { rustTemplate( """ pub(crate) fn new(handle: std::sync::Arc>) -> Self { @@ -162,20 +178,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { } pub async fn send(self) -> Result<#{ok}, #{sdk_err}<#{operation_err}>> where - C: #{tower_service}<#{http}::Request<#{sdk_body}>, Response = #{http}::Response<#{sdk_body}>> + Send + Clone + 'static, - C::Error: Into> + Send + Sync + 'static, - C::Future: Send + 'static, - R: #{nrpolicy}, - M: #{tower_layer}<#{http_tower}::dispatch::DispatchService>, - M::Service: #{tower_service}< - #{opreq}, - Response = #{http}::Response<#{sdk_body}>, - Error = #{http_tower}::SendOperationError, - > + Send - + Clone - + 'static, - >::Future: Send + 'static, - R::Policy: #{tower_retry_Policy}<#{opmod}OperationAlias, #{sdk_ok}<#{ok}>, #{sdk_err}<#{operation_err}>> + Clone, + R::Policy: #{hyper}::bounds::SmithyRetryPolicy<#{input}OperationOutputAlias, #{ok}, #{operation_err}, #{input}OperationRetryAlias>, { let input = self.inner.build().map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; let op = input.make_operation(&self.handle.conf) @@ -183,20 +186,11 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { self.handle.client.call(op).await } """, + "input" to symbolProvider.toSymbol(operation.inputShape(model)), "ok" to symbolProvider.toSymbol(operation.outputShape(model)), - "opmod" to symbolProvider.toSymbol(operation.inputShape(model)), "operation_err" to operation.errorSymbol(symbolProvider), - "sdk_ok" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkSuccess"), "sdk_err" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkError"), - "sdk_body" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "body::SdkBody"), - "opreq" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "operation::Request"), - "op_t" to RuntimeType.operation(runtimeConfig), - "http_tower" to CargoDependency.SmithyHttpTower(runtimeConfig).asType(), - "nrpolicy" to CargoDependency.SmithyHyper(runtimeConfig).asType().copy(name = "retry::NewRequestPolicy"), - "tower_service" to CargoDependency.Tower.asType().copy(name = "Service"), - "tower_layer" to CargoDependency.Tower.asType().copy(name = "layer::Layer"), - "tower_retry_policy" to CargoDependency.Tower.asType().copy(name = "retry::Policy"), - "http" to CargoDependency.Http.asType(), + "hyper" to CargoDependency.SmithyHyper(runtimeConfig).asType(), ) members.forEach { member -> val memberName = symbolProvider.toMemberName(member) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt index 82afb71905..5f11039e9e 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt @@ -72,11 +72,13 @@ abstract class HttpProtocolGenerator( val builderGenerator = BuilderGenerator(model, symbolProvider, operationShape.inputShape(model)) builderGenerator.render(inputWriter) - // TODO: One day, it should be possible for callers to invoke - // buildOperationType directly to get the type rather than depending on - // this alias. - val baseReturnType = buildOperationType(inputWriter, operationShape, customizations) - inputWriter.rust("pub type ${inputShape.id.name}OperationAlias = $baseReturnType;") + // TODO: One day, it should be possible for callers to invoke + // buildOperationType* directly to get the type rather than depending + // on these aliases. + val operationTypeOutput = buildOperationTypeOutput(inputWriter, operationShape) + val operationTypeRetry = buildOperationTypeRetry(inputWriter, customizations) + inputWriter.rust("pub type ${inputShape.id.name}OperationOutputAlias= $operationTypeOutput;") + inputWriter.rust("pub type ${inputShape.id.name}OperationRetryAlias = $operationTypeRetry;") // impl OperationInputShape { ... } inputWriter.implBlock(inputShape, symbolProvider) { @@ -142,11 +144,28 @@ abstract class HttpProtocolGenerator( features: List, ): String { val runtimeConfig = protocolConfig.runtimeConfig - val outputSymbol = symbolProvider.toSymbol(shape) val operationT = RuntimeType.operation(runtimeConfig) + val output = buildOperationTypeOutput(writer, shape) + val retry = buildOperationTypeRetry(writer, features) + + return with(writer) { "${format(operationT)}<$output, $retry>" }; + } + + private fun buildOperationTypeOutput( + writer: RustWriter, + shape: OperationShape, + ): String { + val outputSymbol = symbolProvider.toSymbol(shape) + return with(writer) { "${format(outputSymbol)}" }; + } + + private fun buildOperationTypeRetry( + writer: RustWriter, + features: List, + ): String { val retryType = features.mapNotNull { it.retryType() }.firstOrNull()?.let { writer.format(it) } ?: "()" - return with(writer) { "${format(operationT)}<${format(outputSymbol)}, $retryType>" }; + return with(writer) { "$retryType" }; } private fun buildOperation( diff --git a/rust-runtime/smithy-hyper/src/hyper_impls.rs b/rust-runtime/smithy-hyper/src/hyper_impls.rs index 2967966322..2e03460090 100644 --- a/rust-runtime/smithy-hyper/src/hyper_impls.rs +++ b/rust-runtime/smithy-hyper/src/hyper_impls.rs @@ -47,6 +47,14 @@ impl Builder { } } +#[cfg(feature = "rustls")] +impl crate::Client>, M> { + /// Create a Smithy client that uses HTTPS and the [standard retry policy](retry::Standard). + pub fn new(middleware: M) -> Self { + Builder::new().https().middleware(middleware).build() + } +} + #[cfg(feature = "rustls")] impl Builder { /// Connect to the service over HTTPS using Rustls. diff --git a/rust-runtime/smithy-hyper/src/lib.rs b/rust-runtime/smithy-hyper/src/lib.rs index a13bbb02a4..c020007763 100644 --- a/rust-runtime/smithy-hyper/src/lib.rs +++ b/rust-runtime/smithy-hyper/src/lib.rs @@ -11,7 +11,6 @@ pub mod retry; pub mod test_connection; mod hyper_impls; -use hyper_impls::HyperAdapter; use smithy_http::body::SdkBody; use smithy_http::operation::Operation; @@ -48,13 +47,6 @@ pub struct Client { retry_policy: RetryPolicy, } -impl Client>, M> { - /// Create a Smithy client that uses HTTPS and the [standard retry policy](retry::Standard). - pub fn new(middleware: M) -> Self { - Builder::new().https().middleware(middleware).build() - } -} - impl Client { /// Set the standard retry policy's configuration. pub fn set_retry_config(&mut self, config: retry::Config) { @@ -204,19 +196,9 @@ impl Builder { impl Client where - C: Service, Response = http::Response> + Send + Clone + 'static, - C::Error: Into + Send + Sync + 'static, - C::Future: Send + 'static, + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware, R: retry::NewRequestPolicy, - M: Layer>, - M::Service: Service< - smithy_http::operation::Request, - Response = http::Response, - Error = smithy_http_tower::SendOperationError, - > + Send - + Clone - + 'static, - >::Future: Send + 'static, { /// Dispatch this request to the network /// @@ -224,10 +206,9 @@ where /// access the raw response use `call_raw`. pub async fn call(&self, input: Operation) -> Result> where - O: ParseHttpResponse> + Send + Sync + Clone + 'static, - E: Error + ProvideErrorKind, - R::Policy: tower::retry::Policy, SdkSuccess, SdkError> + Clone, - Retry: ClassifyResponse, SdkError>, + R::Policy: bounds::SmithyRetryPolicy, + bounds::Parsed<>::Service, O, Retry>: + Service, Response = SdkSuccess, Error = SdkError> + Clone, { self.call_raw(input).await.map(|res| res.parsed) } @@ -241,10 +222,15 @@ where input: Operation, ) -> Result, SdkError> where - O: ParseHttpResponse> + Send + Sync + Clone + 'static, - E: Error + ProvideErrorKind, - R::Policy: tower::retry::Policy, SdkSuccess, SdkError> + Clone, - Retry: ClassifyResponse, SdkError>, + R::Policy: bounds::SmithyRetryPolicy, + // This bound is not _technically_ inferred by all the previous bounds, but in practice it + // is because _we_ know that there is only implementation of Service for Parsed + // (ParsedResponseService), and it will apply as long as the bounds on C, M, and R hold, + // and will produce (as expected) Response = SdkSuccess, Error = SdkError. But Rust + // doesn't know that -- there _could_ theoretically be other implementations of Service for + // Parsed that don't return those same types. So, we must give the bound. + bounds::Parsed<>::Service, O, Retry>: + Service, Response = SdkSuccess, Error = SdkError> + Clone, { let connector = self.connector.clone(); let mut svc = ServiceBuilder::new() @@ -255,7 +241,6 @@ where // customer-provided middleware, then dispatch dispatch over the wire. .layer(&self.middleware) .layer(DispatchLayer::new()) - .check_service() .service(connector); svc.ready().await?.call(input).await } @@ -280,6 +265,155 @@ where } } +/// This module holds convenient short-hands for the otherwise fairly extensive trait bounds +/// required for `call` and friends. +/// +/// The short-hands will one day be true [trait aliases], but for now they are traits with blanket +/// implementations. Also, due to [compiler limitations], the bounds repeat a nubmer of associated +/// types with bounds so that those bounds [do not need to be repeated] at the call site. It's a +/// bit of a mess to define, but _should_ be invisible to callers. +/// +/// [trait aliases]: https://rust-lang.github.io/rfcs/1733-trait-alias.html +/// [compiler limitations]: https://github.com/rust-lang/rust/issues/20671 +/// [do not need to be repeated]: https://github.com/rust-lang/rust/issues/20671#issuecomment-529752828 +pub mod bounds { + use super::*; + + /// A service that has parsed a raw Smithy response. + pub type Parsed = + smithy_http_tower::parse_response::ParseResponseService; + + /// A low-level Smithy connector that maps from [`http::Request`] to [`http::Response`]. + /// + /// This trait has a blanket implementation for all compatible types, and should never need to + /// be implemented. + pub trait SmithyConnector: + Service< + http::Request, + Response = http::Response, + Error = ::Error, + Future = ::Future, + > + Send + + Clone + + 'static + { + /// Forwarding type to `::Error` for bound inference. + /// + /// See module-level docs for details. + type Error: Into + Send + Sync + 'static; + + /// Forwarding type to `::Future` for bound inference. + /// + /// See module-level docs for details. + type Future: Send + 'static; + } + + impl SmithyConnector for T + where + T: Service, Response = http::Response> + + Send + + Clone + + 'static, + T::Error: Into + Send + Sync + 'static, + T::Future: Send + 'static, + { + type Error = T::Error; + type Future = T::Future; + } + + /// A Smithy middleware service that adjusts [`smithy_http::operation::Request`]s. + /// + /// This trait has a blanket implementation for all compatible types, and should never need to + /// be implemented. + pub trait SmithyMiddlewareService: + Service< + smithy_http::operation::Request, + Response = http::Response, + Error = smithy_http_tower::SendOperationError, + Future = ::Future, + > + { + /// Forwarding type to `::Future` for bound inference. + /// + /// See module-level docs for details. + type Future: Send + 'static; + } + + impl SmithyMiddlewareService for T + where + T: Service< + smithy_http::operation::Request, + Response = http::Response, + Error = smithy_http_tower::SendOperationError, + >, + T::Future: Send + 'static, + { + type Future = T::Future; + } + + /// A Smithy middleware layer (i.e., factory). + /// + /// This trait has a blanket implementation for all compatible types, and should never need to + /// be implemented. + pub trait SmithyMiddleware: + Layer< + smithy_http_tower::dispatch::DispatchService, + Service = >::Service, + > + { + /// Forwarding type to `::Service` for bound inference. + /// + /// See module-level docs for details. + type Service: SmithyMiddlewareService + Send + Clone + 'static; + } + + impl SmithyMiddleware for T + where + T: Layer>, + T::Service: SmithyMiddlewareService + Send + Clone + 'static, + { + type Service = T::Service; + } + + /// A Smithy retry policy. + /// + /// This trait has a blanket implementation for all compatible types, and should never need to + /// be implemented. + pub trait SmithyRetryPolicy: + tower::retry::Policy, SdkSuccess, SdkError> + Clone + { + /// Forwarding type to `O` for bound inference. + /// + /// See module-level docs for details. + type O: ParseHttpResponse> + + Send + + Sync + + Clone + + 'static; + /// Forwarding type to `E` for bound inference. + /// + /// See module-level docs for details. + type E: Error + ProvideErrorKind; + + /// Forwarding type to `Retry` for bound inference. + /// + /// See module-level docs for details. + type Retry: ClassifyResponse, SdkError>; + } + + impl SmithyRetryPolicy for R + where + R: tower::retry::Policy, SdkSuccess, SdkError> + Clone, + O: ParseHttpResponse> + Send + Sync + Clone + 'static, + E: Error + ProvideErrorKind, + Retry: ClassifyResponse, SdkError>, + { + type O = O; + type E = E; + type Retry = Retry; + } +} + /// This module provides types useful for static tests. /// /// These are only used to write the bounds in [`Client::check`]. Customers will not need them. From 5917587812c109dd40cb830d739560fda19cb56e Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 10:01:05 -0700 Subject: [PATCH 05/36] Make smithy-hyper hyper dep optional --- .../{smithy-hyper => smithy-client}/Cargo.toml | 8 ++++---- rust-runtime/{smithy-hyper => smithy-client}/LICENSE | 0 .../src/hyper_impls.rs | 0 .../{smithy-hyper => smithy-client}/src/lib.rs | 10 ++++++++-- .../{smithy-hyper => smithy-client}/src/retry.rs | 0 .../src/test_connection.rs | 12 ++++++++---- 6 files changed, 20 insertions(+), 10 deletions(-) rename rust-runtime/{smithy-hyper => smithy-client}/Cargo.toml (84%) rename rust-runtime/{smithy-hyper => smithy-client}/LICENSE (100%) rename rust-runtime/{smithy-hyper => smithy-client}/src/hyper_impls.rs (100%) rename rust-runtime/{smithy-hyper => smithy-client}/src/lib.rs (97%) rename rust-runtime/{smithy-hyper => smithy-client}/src/retry.rs (100%) rename rust-runtime/{smithy-hyper => smithy-client}/src/test_connection.rs (95%) diff --git a/rust-runtime/smithy-hyper/Cargo.toml b/rust-runtime/smithy-client/Cargo.toml similarity index 84% rename from rust-runtime/smithy-hyper/Cargo.toml rename to rust-runtime/smithy-client/Cargo.toml index 15e2d6d347..c93e47a04b 100644 --- a/rust-runtime/smithy-hyper/Cargo.toml +++ b/rust-runtime/smithy-client/Cargo.toml @@ -9,12 +9,12 @@ license = "Apache-2.0" [features] test-util = ["protocol-test-helpers"] -default = ["test-util"] -native-tls = ["hyper-tls"] -rustls = ["hyper-rustls"] +default = ["test-util", "hyper"] +native-tls = ["hyper", "hyper-tls"] +rustls = ["hyper", "hyper-rustls"] [dependencies] -hyper = { version = "0.14.2", features = ["client", "http1", "http2", "tcp", "runtime"] } +hyper = { version = "0.14.2", features = ["client", "http2"], optional = true } tower = { version = "0.4.6", features = ["util", "retry"] } hyper-tls = { version ="0.5.0", optional = true } hyper-rustls = { version = "0.22.1", optional = true, features = ["rustls-native-certs"] } diff --git a/rust-runtime/smithy-hyper/LICENSE b/rust-runtime/smithy-client/LICENSE similarity index 100% rename from rust-runtime/smithy-hyper/LICENSE rename to rust-runtime/smithy-client/LICENSE diff --git a/rust-runtime/smithy-hyper/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs similarity index 100% rename from rust-runtime/smithy-hyper/src/hyper_impls.rs rename to rust-runtime/smithy-client/src/hyper_impls.rs diff --git a/rust-runtime/smithy-hyper/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs similarity index 97% rename from rust-runtime/smithy-hyper/src/lib.rs rename to rust-runtime/smithy-client/src/lib.rs index c020007763..c0f6578482 100644 --- a/rust-runtime/smithy-hyper/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -10,6 +10,7 @@ pub mod retry; #[cfg(feature = "test-util")] pub mod test_connection; +#[cfg(feature = "hyper")] mod hyper_impls; use smithy_http::body::SdkBody; @@ -26,7 +27,7 @@ use tower::{Layer, Service, ServiceBuilder, ServiceExt}; type BoxError = Box; -/// Hyper-based Smithy Service Client. +/// Smithy service client. /// /// The service client is customizeable in a number of ways (see [`Builder`]), but most customers /// can stick with the standard constructor provided by [`Client::new`]. It takes only a single @@ -40,6 +41,11 @@ type BoxError = Box; /// [`smithy_http::operation::Request`] and return responses of the type /// [`http::Response`], most likely by modifying the provided request in place, passing it /// to the inner service, and then ultimately returning the inner service's response. +/// +/// With the `hyper` feature enabled, you can construct a `Client` directly from a +/// [`hyper::Client`] using [`Builder::hyper`]. You can also enable the `rustls` or `native-tls` +/// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls`] and +/// [`Builder::native_tls`] respectively. #[derive(Debug)] pub struct Client { connector: Connector, @@ -470,7 +476,7 @@ pub mod static_tests { // Statically check that a hyper client can actually be used to build a Client. #[allow(dead_code)] - #[cfg(test)] + #[cfg(all(test, feature = "hyper"))] fn sanity_hyper(hc: hyper::Client) where C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, diff --git a/rust-runtime/smithy-hyper/src/retry.rs b/rust-runtime/smithy-client/src/retry.rs similarity index 100% rename from rust-runtime/smithy-hyper/src/retry.rs rename to rust-runtime/smithy-client/src/retry.rs diff --git a/rust-runtime/smithy-hyper/src/test_connection.rs b/rust-runtime/smithy-client/src/test_connection.rs similarity index 95% rename from rust-runtime/smithy-hyper/src/test_connection.rs rename to rust-runtime/smithy-client/src/test_connection.rs index 93fa60c1d9..df12c9268c 100644 --- a/rust-runtime/smithy-hyper/src/test_connection.rs +++ b/rust-runtime/smithy-client/src/test_connection.rs @@ -57,7 +57,7 @@ impl ValidateRequest { } } -/// TestConnection for use with a [`smithy_hyper::Client`](crate::Client) +/// TestConnection for use with a [`Client`](crate::Client). /// /// A basic test connection. It will: /// - Response to requests with a preloaded series of responses @@ -109,7 +109,10 @@ impl TestConnection { } } -impl> tower::Service> for TestConnection { +impl tower::Service> for TestConnection +where + SdkBody: From, +{ type Response = http::Response; type Error = BoxError; type Future = Ready>; @@ -125,7 +128,7 @@ impl> tower::Service> for TestConnec .lock() .unwrap() .push(ValidateRequest { expected, actual }); - std::future::ready(Ok(resp.map(|body| SdkBody::from(body.into())))) + std::future::ready(Ok(resp.map(|body| SdkBody::from(body)))) } else { std::future::ready(Err("No more data".into())) } @@ -134,7 +137,8 @@ impl> tower::Service> for TestConnec impl From> for crate::Client, tower::layer::util::Identity> where - B: Into + Send + 'static, + B: Send + 'static, + SdkBody: From, { fn from(tc: TestConnection) -> Self { crate::Builder::new() From 7a3cfa783c6d2925e13ef58c1e0879b0f4c77187 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 10:01:14 -0700 Subject: [PATCH 06/36] Rename smithy-hyper to smithy-client --- aws/rust-runtime/aws-hyper/Cargo.toml | 6 +-- aws/rust-runtime/aws-hyper/src/lib.rs | 8 ++-- aws/sdk/build.gradle.kts | 2 +- .../rust/codegen/rustlang/CargoDependency.kt | 2 +- .../generators/FluentClientDecorator.kt | 42 +++++++++---------- rust-runtime/smithy-client/Cargo.toml | 2 +- rust-runtime/smithy-client/src/lib.rs | 2 +- rust-runtime/smithy-client/src/retry.rs | 2 +- .../smithy-client/src/test_connection.rs | 4 +- 9 files changed, 35 insertions(+), 35 deletions(-) diff --git a/aws/rust-runtime/aws-hyper/Cargo.toml b/aws/rust-runtime/aws-hyper/Cargo.toml index a3c2b01f89..5ee7f8609b 100644 --- a/aws/rust-runtime/aws-hyper/Cargo.toml +++ b/aws/rust-runtime/aws-hyper/Cargo.toml @@ -10,8 +10,8 @@ license = "Apache-2.0" [features] test-util = ["protocol-test-helpers"] default = ["test-util"] -native-tls = ["hyper-tls", "smithy-hyper/native-tls"] -rustls = ["hyper-rustls", "smithy-hyper/rustls"] +native-tls = ["hyper-tls", "smithy-client/native-tls"] +rustls = ["hyper-rustls", "smithy-client/rustls"] [dependencies] hyper = { version = "0.14.2", features = ["client", "http1", "http2", "tcp", "runtime"] } @@ -28,7 +28,7 @@ http-body = "0.4.0" smithy-http = { path = "../../../rust-runtime/smithy-http" } smithy-types = { path = "../../../rust-runtime/smithy-types" } smithy-http-tower = { path = "../../../rust-runtime/smithy-http-tower" } -smithy-hyper = { path = "../../../rust-runtime/smithy-hyper" } +smithy-client = { path = "../../../rust-runtime/smithy-client" } fastrand = "1.4.0" tokio = { version = "1", features = ["time"] } diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index bfc95ed052..1d343aee74 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -7,7 +7,7 @@ pub mod conn; #[cfg(feature = "test-util")] pub mod test_connection; -pub use smithy_hyper::retry::Config as RetryConfig; +pub use smithy_client::retry::Config as RetryConfig; use crate::conn::Standard; use aws_endpoint::AwsEndpointStage; @@ -73,14 +73,14 @@ impl tower::Layer for AwsMiddlewareLayer { /// ``` #[derive(Debug)] pub struct Client { - inner: smithy_hyper::Client, + inner: smithy_client::Client, } impl Client { /// Construct a new `Client` with a custom connector pub fn new(connector: S) -> Self { Client { - inner: smithy_hyper::Builder::new() + inner: smithy_client::Builder::new() .connector(connector) .middleware(AwsMiddlewareLayer) .build(), @@ -98,7 +98,7 @@ impl Client { #[cfg(any(feature = "native-tls", feature = "rustls"))] pub fn https() -> StandardClient { Client { - inner: smithy_hyper::Builder::new() + inner: smithy_client::Builder::new() .connector(Standard::https()) .middleware(AwsMiddlewareLayer) .build(), diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 9a87331327..e96144fa52 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -26,7 +26,7 @@ val runtimeModules = listOf( "smithy-xml", "smithy-http", "smithy-http-tower", - "smithy-hyper", + "smithy-client", "protocol-test-helpers" ) val awsModules = listOf("aws-auth", "aws-endpoint", "aws-types", "aws-hyper", "aws-sig-auth", "aws-http") diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt index 2c7b8ac981..4e42b5b29b 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/CargoDependency.kt @@ -187,7 +187,7 @@ data class CargoDependency( fun SmithyHttp(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("http") fun SmithyHttpTower(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("http-tower") - fun SmithyHyper(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("hyper", true) + fun SmithyClient(runtimeConfig: RuntimeConfig) = runtimeConfig.runtimeCrate("client", true) fun ProtocolTestHelpers(runtimeConfig: RuntimeConfig) = CargoDependency( "protocol-test-helpers", runtimeConfig.runtimeCrateLocation.crateLocation(), scope = DependencyScope.Dev diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index 4a1d6b5f12..1fe63dcf83 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -53,10 +53,10 @@ class FluentClientDecorator : RustCodegenDecorator { rustCrate.withModule(RustModule("client", module)) { writer -> FluentClientGenerator(protocolConfig).render(writer) } - val smithyHyper = CargoDependency.SmithyHyper(protocolConfig.runtimeConfig) - rustCrate.addFeature(Feature("client", true, listOf(smithyHyper.name))) - rustCrate.addFeature(Feature("rustls", default = true, listOf("smithy-hyper/rustls"))) - rustCrate.addFeature(Feature("native-tls", default = false, listOf("smithy-hyper/native-tls"))) + val smithyClient = CargoDependency.SmithyClient(protocolConfig.runtimeConfig) + rustCrate.addFeature(Feature("client", true, listOf(smithyClient.name))) + rustCrate.addFeature(Feature("rustls", default = true, listOf("smithy-client/rustls"))) + rustCrate.addFeature(Feature("native-tls", default = false, listOf("smithy-client/native-tls"))) } override fun libRsCustomizations( @@ -85,7 +85,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { TopDownIndex.of(protocolConfig.model).getContainedOperations(serviceShape).sortedBy { it.id } private val symbolProvider = protocolConfig.symbolProvider private val model = protocolConfig.model - private val hyperDep = CargoDependency.SmithyHyper(protocolConfig.runtimeConfig).copy(optional = true) + private val clientDep = CargoDependency.SmithyClient(protocolConfig.runtimeConfig).copy(optional = true) private val runtimeConfig = protocolConfig.runtimeConfig fun render(writer: RustWriter) { @@ -93,23 +93,23 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { """ ##[derive(std::fmt::Debug)] pub(crate) struct Handle { - client: #{hyper}::Client, + client: #{client}::Client, conf: crate::Config, } ##[derive(Clone, std::fmt::Debug)] - pub struct Client { + pub struct Client { handle: std::sync::Arc> } - impl From<#{hyper}::Client> for Client { - fn from(client: #{hyper}::Client) -> Self { + impl From<#{client}::Client> for Client { + fn from(client: #{client}::Client) -> Self { Self::with_config(client, crate::Config::builder().build()) } } impl Client { - pub fn with_config(client: #{hyper}::Client, conf: crate::Config) -> Self { + pub fn with_config(client: #{client}::Client, conf: crate::Config) -> Self { Self { handle: std::sync::Arc::new(Handle { client, @@ -123,17 +123,17 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { } } """, - "hyper" to hyperDep.asType() + "client" to clientDep.asType() ) writer.rustBlockTemplate( """ impl Client where - C: #{hyper}::bounds::SmithyConnector, - M: #{hyper}::bounds::SmithyMiddleware, - R: #{hyper}::retry::NewRequestPolicy, + C: #{client}::bounds::SmithyConnector, + M: #{client}::bounds::SmithyMiddleware, + R: #{client}::retry::NewRequestPolicy, """, - "hyper" to hyperDep.asType(), + "client" to clientDep.asType(), ) { operations.forEach { operation -> val name = symbolProvider.toSymbol(operation).name @@ -165,11 +165,11 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { """ impl $name where - C: #{hyper}::bounds::SmithyConnector, - M: #{hyper}::bounds::SmithyMiddleware, - R: #{hyper}::retry::NewRequestPolicy, + C: #{client}::bounds::SmithyConnector, + M: #{client}::bounds::SmithyMiddleware, + R: #{client}::retry::NewRequestPolicy, """, - "hyper" to CargoDependency.SmithyHyper(runtimeConfig).asType(), + "client" to CargoDependency.SmithyClient(runtimeConfig).asType(), ) { rustTemplate( """ @@ -178,7 +178,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { } pub async fn send(self) -> Result<#{ok}, #{sdk_err}<#{operation_err}>> where - R::Policy: #{hyper}::bounds::SmithyRetryPolicy<#{input}OperationOutputAlias, #{ok}, #{operation_err}, #{input}OperationRetryAlias>, + R::Policy: #{client}::bounds::SmithyRetryPolicy<#{input}OperationOutputAlias, #{ok}, #{operation_err}, #{input}OperationRetryAlias>, { let input = self.inner.build().map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; let op = input.make_operation(&self.handle.conf) @@ -190,7 +190,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { "ok" to symbolProvider.toSymbol(operation.outputShape(model)), "operation_err" to operation.errorSymbol(symbolProvider), "sdk_err" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkError"), - "hyper" to CargoDependency.SmithyHyper(runtimeConfig).asType(), + "client" to CargoDependency.SmithyClient(runtimeConfig).asType(), ) members.forEach { member -> val memberName = symbolProvider.toMemberName(member) diff --git a/rust-runtime/smithy-client/Cargo.toml b/rust-runtime/smithy-client/Cargo.toml index c93e47a04b..8993847f39 100644 --- a/rust-runtime/smithy-client/Cargo.toml +++ b/rust-runtime/smithy-client/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "smithy-hyper" +name = "smithy-client" version = "0.1.0" authors = ["Russell Cohen "] edition = "2018" diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index c0f6578482..091a6370bc 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -165,7 +165,7 @@ impl Builder { /// Use a connector that directly maps each request to a response. /// /// ```rust - /// use smithy_hyper::Builder; + /// use smithy_client::Builder; /// use smithy_http::body::SdkBody; /// let client = Builder::new() /// # /* diff --git a/rust-runtime/smithy-client/src/retry.rs b/rust-runtime/smithy-client/src/retry.rs index f982225215..38cc1d831e 100644 --- a/rust-runtime/smithy-client/src/retry.rs +++ b/rust-runtime/smithy-client/src/retry.rs @@ -62,7 +62,7 @@ impl Config { /// By default, `base` is a randomly generated value between 0 and 1. In tests, it can /// be helpful to override this: /// ```rust - /// use smithy_hyper::retry::Config; + /// use smithy_client::retry::Config; /// let conf = Config::default().with_base(||1_f64); /// ``` pub fn with_base(mut self, base: fn() -> f64) -> Self { diff --git a/rust-runtime/smithy-client/src/test_connection.rs b/rust-runtime/smithy-client/src/test_connection.rs index df12c9268c..8d37ee6a08 100644 --- a/rust-runtime/smithy-client/src/test_connection.rs +++ b/rust-runtime/smithy-client/src/test_connection.rs @@ -67,7 +67,7 @@ impl ValidateRequest { /// For more complex use cases, see [Tower Test](https://docs.rs/tower-test/0.4.0/tower_test/) /// Usage example: /// ```rust -/// use smithy_hyper::test_connection::TestConnection; +/// use smithy_client::test_connection::TestConnection; /// use smithy_http::body::SdkBody; /// let events = vec![( /// http::Request::new(SdkBody::from("request body")), @@ -77,7 +77,7 @@ impl ValidateRequest { /// .unwrap(), /// )]; /// let conn = TestConnection::new(events); -/// let client = smithy_hyper::Client::from(conn); +/// let client = smithy_client::Client::from(conn); /// ``` #[derive(Debug)] pub struct TestConnection { From a2181cd9ff04756118bb454aabd0a1f87442693c Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 11:19:00 -0700 Subject: [PATCH 07/36] Enable rustls by default --- rust-runtime/smithy-client/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/smithy-client/Cargo.toml b/rust-runtime/smithy-client/Cargo.toml index 8993847f39..6686169f3e 100644 --- a/rust-runtime/smithy-client/Cargo.toml +++ b/rust-runtime/smithy-client/Cargo.toml @@ -9,7 +9,7 @@ license = "Apache-2.0" [features] test-util = ["protocol-test-helpers"] -default = ["test-util", "hyper"] +default = ["test-util", "hyper", "rustls"] native-tls = ["hyper", "hyper-tls"] rustls = ["hyper", "hyper-rustls"] From 2f2305029e1af53a6532549ce264c0774aa9a488 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 11:19:14 -0700 Subject: [PATCH 08/36] Also warn on rust 2018 idioms --- rust-runtime/smithy-client/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index 091a6370bc..223b81b7b4 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -3,7 +3,12 @@ * SPDX-License-Identifier: Apache-2.0. */ //! A Hyper-based Smithy service client. -#![warn(missing_debug_implementations, missing_docs, rustdoc::all)] +#![warn( + missing_debug_implementations, + missing_docs, + rustdoc::all, + rust_2018_idioms +)] pub mod retry; From 10ad30d080a2da0ebd74deaa8300794fd302ae25 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 11:19:29 -0700 Subject: [PATCH 09/36] Add type-erased middleware --- rust-runtime/smithy-client/src/erase.rs | 151 ++++++++++++++++++ rust-runtime/smithy-client/src/lib.rs | 117 +++++++++++++- .../smithy-http-tower/src/dispatch.rs | 10 +- 3 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 rust-runtime/smithy-client/src/erase.rs diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs new file mode 100644 index 0000000000..14535269e6 --- /dev/null +++ b/rust-runtime/smithy-client/src/erase.rs @@ -0,0 +1,151 @@ +// This is an adaptation of tower::util::{BoxLayer, BoxService} that includes Clone and doesn't +// include Sync. + +use std::fmt; +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; +use tower::layer::{layer_fn, Layer}; +use tower::Service; + +pub(super) struct BoxCloneLayer { + boxed: Box>>, +} + +impl BoxCloneLayer { + /// Create a new [`BoxLayer`]. + pub fn new(inner_layer: L) -> Self + where + L: Layer + Send + 'static, + L::Service: Service + Clone + Send + 'static, + >::Future: Send + 'static, + { + let layer = layer_fn(move |inner: In| { + let out = inner_layer.layer(inner); + BoxCloneService::new(out) + }); + + Self { + boxed: Box::new(layer), + } + } +} + +impl Layer for BoxCloneLayer { + type Service = BoxCloneService; + + fn layer(&self, inner: In) -> Self::Service { + self.boxed.layer(inner) + } +} + +impl fmt::Debug for BoxCloneLayer { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("BoxCloneLayer").finish() + } +} + +trait CloneService: Service { + fn clone_box( + &self, + ) -> Box< + dyn CloneService + + Send + + 'static, + >; +} + +impl CloneService for T +where + T: Service + Clone + Send + 'static, +{ + fn clone_box( + &self, + ) -> Box< + dyn CloneService< + Request, + Response = Self::Response, + Error = Self::Error, + Future = Self::Future, + > + + 'static + + Send, + > { + Box::new(self.clone()) + } +} + +type BoxFuture = Pin> + Send>>; +pub struct BoxCloneService { + inner: Box< + dyn CloneService> + Send + 'static, + >, +} + +#[derive(Debug, Clone)] +struct Boxed { + inner: S, +} + +impl BoxCloneService { + #[allow(missing_docs)] + pub fn new(inner: S) -> Self + where + S: Service + Send + 'static + Clone, + S::Future: Send + 'static, + { + let inner = Box::new(Boxed { inner }); + BoxCloneService { inner } + } +} + +impl Clone for BoxCloneService +where + T: 'static, + U: 'static, + E: 'static, +{ + fn clone(&self) -> Self { + Self { + inner: self.clone_box(), + } + } +} + +impl Service for BoxCloneService { + type Response = U; + type Error = E; + type Future = BoxFuture; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, request: T) -> BoxFuture { + self.inner.call(request) + } +} + +impl fmt::Debug for BoxCloneService { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("BoxCloneService").finish() + } +} + +impl Service for Boxed +where + S: Service + 'static, + S::Future: Send + 'static, +{ + type Response = S::Response; + type Error = S::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, request: Request) -> Self::Future { + Box::pin(self.inner.call(request)) + } +} diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index 223b81b7b4..f280bf7fa2 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -18,6 +18,25 @@ pub mod test_connection; #[cfg(feature = "hyper")] mod hyper_impls; +/// Type aliases for standard connection types. +#[cfg(feature = "hyper")] +#[allow(missing_docs)] +pub mod conns { + #[cfg(feature = "rustls")] + pub type Https = crate::hyper_impls::HyperAdapter< + hyper_rustls::HttpsConnector, + >; + + #[cfg(feature = "native-tls")] + pub type NativeTls = + crate::hyper_impls::HyperAdapter>; + + #[cfg(feature = "rustls")] + pub type Rustls = crate::hyper_impls::HyperAdapter< + hyper_rustls::HttpsConnector, + >; +} + use smithy_http::body::SdkBody; use smithy_http::operation::Operation; use smithy_http::response::ParseHttpResponse; @@ -27,7 +46,7 @@ use smithy_http_tower::dispatch::DispatchLayer; use smithy_http_tower::parse_response::ParseResponseLayer; use smithy_types::retry::ProvideErrorKind; use std::error::Error; -use std::fmt::Debug; +use std::fmt; use tower::{Layer, Service, ServiceBuilder, ServiceExt}; type BoxError = Box; @@ -52,7 +71,7 @@ type BoxError = Box; /// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls`] and /// [`Builder::native_tls`] respectively. #[derive(Debug)] -pub struct Client { +pub struct Client, RetryPolicy = retry::Standard> { connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, @@ -276,6 +295,89 @@ where } } +impl Client +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, +{ + /// Erase the middleware type from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynMiddleware`] for details. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.simplify() }; + /// # client.client.check(); + /// # } + pub fn simplify(self) -> Client, R> { + Client { + connector: self.connector, + middleware: DynMiddleware::new(self.middleware), + retry_policy: self.retry_policy, + } + } +} + +// These types are technically public in that they're reachable from the public trait impls on +// DynMiddleware, but no-one should ever look at them or use them. +#[doc(hidden)] +pub mod erase; + +/// A Smithy middleware that uses dynamic dispatch. +/// +/// This type allows you to pay a small runtime cost to avoid having to name the exact middleware +/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to +/// enable dynamic dispatch for every request that goes through the middleware, which increases +/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely +/// to matter in all but the highest-performance settings. +pub struct DynMiddleware( + erase::BoxCloneLayer< + smithy_http_tower::dispatch::DispatchService, + smithy_http::operation::Request, + http::Response, + smithy_http_tower::SendOperationError, + >, +); + +impl fmt::Debug for DynMiddleware { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("DynMiddleware").finish() + } +} + +impl DynMiddleware { + /// Construct a new dynamically-dispatched Smithy middleware. + pub fn new + Send + 'static>(middleware: M) -> Self { + Self(erase::BoxCloneLayer::new(middleware)) + } +} + +impl Layer> for DynMiddleware { + type Service = erase::BoxCloneService< + smithy_http::operation::Request, + http::Response, + smithy_http_tower::SendOperationError, + >; + + fn layer(&self, inner: smithy_http_tower::dispatch::DispatchService) -> Self::Service { + self.0.layer(inner) + } +} + /// This module holds convenient short-hands for the otherwise fairly extensive trait bounds /// required for `call` and friends. /// @@ -492,4 +594,15 @@ pub mod static_tests { .build() .check(); } + + // Statically check that a type-erased client (dynclient) is actually a valid Client. + #[allow(dead_code)] + fn sanity_simplify() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .simplify() + .check(); + } } diff --git a/rust-runtime/smithy-http-tower/src/dispatch.rs b/rust-runtime/smithy-http-tower/src/dispatch.rs index 481151eaab..06eb5297b5 100644 --- a/rust-runtime/smithy-http-tower/src/dispatch.rs +++ b/rust-runtime/smithy-http-tower/src/dispatch.rs @@ -25,7 +25,7 @@ type BoxedResultFuture = Pin> + Send> impl Service for DispatchService where - S: Service> + Clone + Send + 'static, + S: Service> + Send + 'static, S::Error: Into, S::Future: Send + 'static, { @@ -41,12 +41,10 @@ where fn call(&mut self, req: operation::Request) -> Self::Future { let (req, _property_bag) = req.into_parts(); - let mut inner = self.inner.clone(); + trace!(request = ?req); + let fut = self.inner.call(req); let future = async move { - trace!(request = ?req); - inner - .call(req) - .await + fut.await .map_err(|e| SendOperationError::RequestDispatchError(e.into())) }; Box::pin(future) From 65427b0085f03e7c8361c7a0f3983ac246cdcef5 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 12:01:38 -0700 Subject: [PATCH 10/36] Restore old DispatchLayer tracing --- rust-runtime/smithy-http-tower/src/dispatch.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/rust-runtime/smithy-http-tower/src/dispatch.rs b/rust-runtime/smithy-http-tower/src/dispatch.rs index 06eb5297b5..481151eaab 100644 --- a/rust-runtime/smithy-http-tower/src/dispatch.rs +++ b/rust-runtime/smithy-http-tower/src/dispatch.rs @@ -25,7 +25,7 @@ type BoxedResultFuture = Pin> + Send> impl Service for DispatchService where - S: Service> + Send + 'static, + S: Service> + Clone + Send + 'static, S::Error: Into, S::Future: Send + 'static, { @@ -41,10 +41,12 @@ where fn call(&mut self, req: operation::Request) -> Self::Future { let (req, _property_bag) = req.into_parts(); - trace!(request = ?req); - let fut = self.inner.call(req); + let mut inner = self.inner.clone(); let future = async move { - fut.await + trace!(request = ?req); + inner + .call(req) + .await .map_err(|e| SendOperationError::RequestDispatchError(e.into())) }; Box::pin(future) From bfde222e12a51faee4357081c7eaef42d04b8ef7 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 12:29:41 -0700 Subject: [PATCH 11/36] Add connection type-erasure --- rust-runtime/smithy-client/src/erase.rs | 2 +- rust-runtime/smithy-client/src/lib.rs | 179 +++++++++++++++++++++++- 2 files changed, 174 insertions(+), 7 deletions(-) diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs index 14535269e6..d29a652801 100644 --- a/rust-runtime/smithy-client/src/erase.rs +++ b/rust-runtime/smithy-client/src/erase.rs @@ -75,7 +75,7 @@ where } } -type BoxFuture = Pin> + Send>>; +pub type BoxFuture = Pin> + Send>>; pub struct BoxCloneService { inner: Box< dyn CloneService> + Send + 'static, diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index f280bf7fa2..66a31482b2 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -71,7 +71,11 @@ type BoxError = Box; /// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls`] and /// [`Builder::native_tls`] respectively. #[derive(Debug)] -pub struct Client, RetryPolicy = retry::Standard> { +pub struct Client< + Connector = DynConnector, + Middleware = DynMiddleware, + RetryPolicy = retry::Standard, +> { connector: Connector, middleware: Middleware, retry_policy: RetryPolicy, @@ -224,6 +228,37 @@ impl Builder { } } +impl Builder +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, +{ + /// Build a type-erased Smithy service [`Client`]. + /// + /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` + /// is equivalent to [`Client`] with no type arguments. + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: smithy_client::Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build_dyn(); + /// let client = MyClient { client }; + /// # client.client.check(); + /// # } + pub fn build_dyn(self) -> DynClient { + self.build().simplify() + } +} + impl Client where C: bounds::SmithyConnector, @@ -320,10 +355,10 @@ where /// .https() /// .middleware(tower::layer::util::Identity::new()) /// .build(); - /// let client = MyClient { client: client.simplify() }; + /// let client = MyClient { client: client.erase_middleware() }; /// # client.client.check(); /// # } - pub fn simplify(self) -> Client, R> { + pub fn erase_middleware(self) -> Client, R> { Client { connector: self.connector, middleware: DynMiddleware::new(self.middleware), @@ -332,11 +367,131 @@ where } } +impl Client +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, +{ + /// Erase the connector type from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynConnector`] for details. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// # type MyMiddleware = smithy_client::DynMiddleware; + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.erase_connector() }; + /// # client.client.check(); + /// # } + pub fn erase_connector(self) -> Client { + Client { + connector: DynConnector::new(self.connector), + middleware: self.middleware, + retry_policy: self.retry_policy, + } + } + + /// Erase the connector and middleware types from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynConnector`] and [`DynMiddleware`] for details. + /// + /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` + /// is equivalent to `Client` with no type arguments. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: smithy_client::Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.simplify() }; + /// # client.client.check(); + /// # } + pub fn simplify(self) -> DynClient { + self.erase_connector().erase_middleware() + } +} + // These types are technically public in that they're reachable from the public trait impls on // DynMiddleware, but no-one should ever look at them or use them. #[doc(hidden)] pub mod erase; +/// A [`Client`] whose connector and middleware types have been erased. +/// +/// Mainly useful if you need to name `R` in a type-erased client. If you do not, you can instead +/// just use `Client` with no type parameters, which ends up being the same type. +pub type DynClient = Client, R>; + +/// A Smithy connector that uses dynamic dispatch. +/// +/// This type allows you to pay a small runtime cost to avoid having to name the exact connector +/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to +/// enable dynamic dispatch for every request that goes through the connector, which increases +/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely +/// to matter in all but the highest-performance settings. +#[non_exhaustive] +#[derive(Clone)] +pub struct DynConnector( + erase::BoxCloneService, http::Response, BoxError>, +); + +impl fmt::Debug for DynConnector { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("DynConnector").finish() + } +} + +impl DynConnector { + /// Construct a new dynamically-dispatched Smithy middleware. + pub fn new(connector: C) -> Self + where + C: bounds::SmithyConnector + Send + 'static, + E: Into, + { + Self(erase::BoxCloneService::new(connector.map_err(|e| e.into()))) + } +} + +impl Service> for DynConnector { + type Response = http::Response; + type Error = BoxError; + type Future = erase::BoxFuture; + + fn poll_ready( + &mut self, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.0.poll_ready(cx) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + self.0.call(req) + } +} + /// A Smithy middleware that uses dynamic dispatch. /// /// This type allows you to pay a small runtime cost to avoid having to name the exact middleware @@ -344,6 +499,7 @@ pub mod erase; /// enable dynamic dispatch for every request that goes through the middleware, which increases /// memory pressure and suffers an additional vtable indirection for each request, but is unlikely /// to matter in all but the highest-performance settings. +#[non_exhaustive] pub struct DynMiddleware( erase::BoxCloneLayer< smithy_http_tower::dispatch::DispatchService, @@ -595,14 +751,25 @@ pub mod static_tests { .check(); } - // Statically check that a type-erased client (dynclient) is actually a valid Client. + // Statically check that a type-erased middleware client is actually a valid Client. + #[allow(dead_code)] + fn sanity_erase_middleware() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .erase_middleware() + .check(); + } + + // Statically check that a type-erased connector client is actually a valid Client. #[allow(dead_code)] - fn sanity_simplify() { + fn sanity_erase_connector() { Builder::new() .middleware(tower::layer::util::Identity::new()) .map_connector(|_| async { unreachable!() }) .build() - .simplify() + .erase_connector() .check(); } } From fdeed6d0c4799acdfcd3d35fd359743d81e79d1d Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 13:25:28 -0700 Subject: [PATCH 12/36] Fix rustdoc link --- rust-runtime/smithy-client/src/hyper_impls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index 2e03460090..95dc76ec75 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -49,7 +49,8 @@ impl Builder { #[cfg(feature = "rustls")] impl crate::Client>, M> { - /// Create a Smithy client that uses HTTPS and the [standard retry policy](retry::Standard). + /// Create a Smithy client that uses HTTPS and the [standard retry + /// policy](crate::retry::Standard). pub fn new(middleware: M) -> Self { Builder::new().https().middleware(middleware).build() } From 2f3d664004c330f4f8e527acdb5825f5a748bd3a Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 14:02:30 -0700 Subject: [PATCH 13/36] Split up lib.rs --- rust-runtime/smithy-client/src/bounds.rs | 139 ++++ rust-runtime/smithy-client/src/builder.rs | 173 +++++ rust-runtime/smithy-client/src/erase.rs | 290 ++++---- .../smithy-client/src/erase/boxclone.rs | 151 +++++ rust-runtime/smithy-client/src/lib.rs | 631 +----------------- .../smithy-client/src/static_tests.rs | 85 +++ 6 files changed, 735 insertions(+), 734 deletions(-) create mode 100644 rust-runtime/smithy-client/src/bounds.rs create mode 100644 rust-runtime/smithy-client/src/builder.rs create mode 100644 rust-runtime/smithy-client/src/erase/boxclone.rs create mode 100644 rust-runtime/smithy-client/src/static_tests.rs diff --git a/rust-runtime/smithy-client/src/bounds.rs b/rust-runtime/smithy-client/src/bounds.rs new file mode 100644 index 0000000000..2c90869e28 --- /dev/null +++ b/rust-runtime/smithy-client/src/bounds.rs @@ -0,0 +1,139 @@ +//! This module holds convenient short-hands for the otherwise fairly extensive trait bounds +//! required for `call` and friends. +//! +//! The short-hands will one day be true [trait aliases], but for now they are traits with blanket +//! implementations. Also, due to [compiler limitations], the bounds repeat a nubmer of associated +//! types with bounds so that those bounds [do not need to be repeated] at the call site. It's a +//! bit of a mess to define, but _should_ be invisible to callers. +//! +//! [trait aliases]: https://rust-lang.github.io/rfcs/1733-trait-alias.html +//! [compiler limitations]: https://github.com/rust-lang/rust/issues/20671 +//! [do not need to be repeated]: https://github.com/rust-lang/rust/issues/20671#issuecomment-529752828 + +use crate::*; + +/// A service that has parsed a raw Smithy response. +pub type Parsed = smithy_http_tower::parse_response::ParseResponseService; + +/// A low-level Smithy connector that maps from [`http::Request`] to [`http::Response`]. +/// +/// This trait has a blanket implementation for all compatible types, and should never need to +/// be implemented. +pub trait SmithyConnector: + Service< + http::Request, + Response = http::Response, + Error = ::Error, + Future = ::Future, + > + Send + + Clone + + 'static +{ + /// Forwarding type to `::Error` for bound inference. + /// + /// See module-level docs for details. + type Error: Into + Send + Sync + 'static; + + /// Forwarding type to `::Future` for bound inference. + /// + /// See module-level docs for details. + type Future: Send + 'static; +} + +impl SmithyConnector for T +where + T: Service, Response = http::Response> + Send + Clone + 'static, + T::Error: Into + Send + Sync + 'static, + T::Future: Send + 'static, +{ + type Error = T::Error; + type Future = T::Future; +} + +/// A Smithy middleware service that adjusts [`smithy_http::operation::Request`]s. +/// +/// This trait has a blanket implementation for all compatible types, and should never need to +/// be implemented. +pub trait SmithyMiddlewareService: + Service< + smithy_http::operation::Request, + Response = http::Response, + Error = smithy_http_tower::SendOperationError, + Future = ::Future, +> +{ + /// Forwarding type to `::Future` for bound inference. + /// + /// See module-level docs for details. + type Future: Send + 'static; +} + +impl SmithyMiddlewareService for T +where + T: Service< + smithy_http::operation::Request, + Response = http::Response, + Error = smithy_http_tower::SendOperationError, + >, + T::Future: Send + 'static, +{ + type Future = T::Future; +} + +/// A Smithy middleware layer (i.e., factory). +/// +/// This trait has a blanket implementation for all compatible types, and should never need to +/// be implemented. +pub trait SmithyMiddleware: + Layer< + smithy_http_tower::dispatch::DispatchService, + Service = >::Service, +> +{ + /// Forwarding type to `::Service` for bound inference. + /// + /// See module-level docs for details. + type Service: SmithyMiddlewareService + Send + Clone + 'static; +} + +impl SmithyMiddleware for T +where + T: Layer>, + T::Service: SmithyMiddlewareService + Send + Clone + 'static, +{ + type Service = T::Service; +} + +/// A Smithy retry policy. +/// +/// This trait has a blanket implementation for all compatible types, and should never need to +/// be implemented. +pub trait SmithyRetryPolicy: + tower::retry::Policy, SdkSuccess, SdkError> + Clone +{ + /// Forwarding type to `O` for bound inference. + /// + /// See module-level docs for details. + type O: ParseHttpResponse> + Send + Sync + Clone + 'static; + /// Forwarding type to `E` for bound inference. + /// + /// See module-level docs for details. + type E: Error + ProvideErrorKind; + + /// Forwarding type to `Retry` for bound inference. + /// + /// See module-level docs for details. + type Retry: ClassifyResponse, SdkError>; +} + +impl SmithyRetryPolicy for R +where + R: tower::retry::Policy, SdkSuccess, SdkError> + Clone, + O: ParseHttpResponse> + Send + Sync + Clone + 'static, + E: Error + ProvideErrorKind, + Retry: ClassifyResponse, SdkError>, +{ + type O = O; + type E = E; + type Retry = Retry; +} diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs new file mode 100644 index 0000000000..1f0ca98b91 --- /dev/null +++ b/rust-runtime/smithy-client/src/builder.rs @@ -0,0 +1,173 @@ +use crate::{bounds, erase, retry, BoxError, Client}; +use smithy_http::body::SdkBody; + +/// A builder that provides more customization options when constructing a [`Client`]. +/// +/// To start, call [`Builder::new`]. Then, chain the method calls to configure the `Builder`. +/// When configured to your liking, call [`Builder::build`]. The individual methods have additional +/// documentation. +#[derive(Clone, Debug)] +pub struct Builder { + connector: C, + middleware: M, + retry_policy: R, +} + +impl Default for Builder<(), ()> { + fn default() -> Self { + Builder { + connector: (), + middleware: (), + retry_policy: retry::Standard::default(), + } + } +} + +impl Builder<(), ()> { + /// Construct a new, unconfigured builder. + /// + /// This builder cannot yet be used, as it does not specify a [connector](Builder::connector) + /// or [middleware](Builder::middleware). It uses the [standard retry + /// mechanism](retry::Standard). + pub fn new() -> Self { + Self::default() + } +} + +impl Builder { + /// Specify the connector for the eventual client to use. + /// + /// The connector dictates how requests are turned into responses. Normally, this would entail + /// sending the request to some kind of remote server, but in certain settings it's useful to + /// be able to use a custom connector instead, such as to mock the network for tests. + /// + /// If you want to use a custom hyper connector, use [`Builder::hyper`]. + /// + /// If you just want to specify a function from request to response instead, use + /// [`Builder::map_connector`]. + pub fn connector(self, connector: C2) -> Builder { + Builder { + connector, + retry_policy: self.retry_policy, + middleware: self.middleware, + } + } + + /// Specify the middleware for the eventual client ot use. + /// + /// The middleware adjusts requests before they are dispatched to the connector. It is + /// responsible for filling in any request parameters that aren't specified by the Smithy + /// protocol definition, such as those used for routing (like the URL), authentication, and + /// authorization. + /// + /// The middleware takes the form of a [`tower::Layer`] that wraps the actual connection for + /// each request. The [`tower::Service`] that the middleware produces must accept requests of + /// the type [`smithy_http::operation::Request`] and return responses of the type + /// [`http::Response`], most likely by modifying the provided request in place, + /// passing it to the inner service, and then ultimately returning the inner service's + /// response. + /// + /// If your requests are already ready to be sent and need no adjustment, you can use + /// [`tower::layer::util::Identity`] as your middleware. + pub fn middleware(self, middleware: M2) -> Builder { + Builder { + connector: self.connector, + retry_policy: self.retry_policy, + middleware, + } + } + + /// Specify the retry policy for the eventual client to use. + /// + /// By default, the Smithy client uses a standard retry policy that works well in most + /// settings. You can use this method to override that policy with a custom one. A new policy + /// instance will be instantiated for each request using [`retry::NewRequestPolicy`]. Each + /// policy instance must implement [`tower::retry::Policy`]. + /// + /// If you just want to modify the policy _configuration_ for the standard retry policy, use + /// [`Builder::set_retry_config`]. + pub fn retry_policy(self, retry_policy: R2) -> Builder { + Builder { + connector: self.connector, + retry_policy, + middleware: self.middleware, + } + } +} + +impl Builder { + /// Set the standard retry policy's configuration. + pub fn set_retry_config(&mut self, config: retry::Config) { + self.retry_policy.with_config(config); + } +} + +impl Builder { + /// Use a connector that directly maps each request to a response. + /// + /// ```rust + /// use smithy_client::Builder; + /// use smithy_http::body::SdkBody; + /// let client = Builder::new() + /// # /* + /// .middleware(..) + /// # */ + /// # .middleware(tower::layer::util::Identity::new()) + /// .map_connector(|req: http::Request| { + /// async move { + /// Ok(http::Response::new(SdkBody::empty())) + /// } + /// }) + /// .build(); + /// # client.check(); + /// ``` + pub fn map_connector(self, map: F) -> Builder, M, R> + where + F: Fn(http::Request) -> FF + Send, + FF: std::future::Future, BoxError>>, + { + self.connector(tower::service_fn(map)) + } +} + +impl Builder { + /// Build a Smithy service [`Client`]. + pub fn build(self) -> Client { + Client { + connector: self.connector, + retry_policy: self.retry_policy, + middleware: self.middleware, + } + } +} + +impl Builder +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, +{ + /// Build a type-erased Smithy service [`Client`]. + /// + /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` + /// is equivalent to [`Client`] with no type arguments. + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: smithy_client::Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build_dyn(); + /// let client = MyClient { client }; + /// # client.client.check(); + /// # } + pub fn build_dyn(self) -> erase::DynClient { + self.build().simplify() + } +} diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs index d29a652801..489ac8437f 100644 --- a/rust-runtime/smithy-client/src/erase.rs +++ b/rust-runtime/smithy-client/src/erase.rs @@ -1,151 +1,209 @@ -// This is an adaptation of tower::util::{BoxLayer, BoxService} that includes Clone and doesn't -// include Sync. +//! Type-erased variants of [`Client`] and friends. +// These types are technically public in that they're reachable from the public trait impls on +// DynMiddleware, but no-one should ever look at them or use them. +#[doc(hidden)] +pub mod boxclone; +use boxclone::*; + +use crate::{bounds, retry, BoxError, Client}; +use smithy_http::body::SdkBody; use std::fmt; -use std::future::Future; -use std::pin::Pin; -use std::task::{Context, Poll}; -use tower::layer::{layer_fn, Layer}; -use tower::Service; - -pub(super) struct BoxCloneLayer { - boxed: Box>>, -} +use tower::{Layer, Service, ServiceExt}; -impl BoxCloneLayer { - /// Create a new [`BoxLayer`]. - pub fn new(inner_layer: L) -> Self - where - L: Layer + Send + 'static, - L::Service: Service + Clone + Send + 'static, - >::Future: Send + 'static, - { - let layer = layer_fn(move |inner: In| { - let out = inner_layer.layer(inner); - BoxCloneService::new(out) - }); +/// A [`Client`] whose connector and middleware types have been erased. +/// +/// Mainly useful if you need to name `R` in a type-erased client. If you do not, you can instead +/// just use `Client` with no type parameters, which ends up being the same type. +pub type DynClient = Client, R>; - Self { - boxed: Box::new(layer), +impl Client +where + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, +{ + /// Erase the middleware type from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynMiddleware`] for details. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.erase_middleware() }; + /// # client.client.check(); + /// # } + pub fn erase_middleware(self) -> Client, R> { + Client { + connector: self.connector, + middleware: DynMiddleware::new(self.middleware), + retry_policy: self.retry_policy, } } } -impl Layer for BoxCloneLayer { - type Service = BoxCloneService; - - fn layer(&self, inner: In) -> Self::Service { - self.boxed.layer(inner) - } -} - -impl fmt::Debug for BoxCloneLayer { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("BoxCloneLayer").finish() - } -} - -trait CloneService: Service { - fn clone_box( - &self, - ) -> Box< - dyn CloneService - + Send - + 'static, - >; -} - -impl CloneService for T +impl Client where - T: Service + Clone + Send + 'static, + C: bounds::SmithyConnector, + M: bounds::SmithyMiddleware + Send + 'static, + R: retry::NewRequestPolicy, { - fn clone_box( - &self, - ) -> Box< - dyn CloneService< - Request, - Response = Self::Response, - Error = Self::Error, - Future = Self::Future, - > - + 'static - + Send, - > { - Box::new(self.clone()) + /// Erase the connector type from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynConnector`] for details. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// # type MyMiddleware = smithy_client::DynMiddleware; + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.erase_connector() }; + /// # client.client.check(); + /// # } + pub fn erase_connector(self) -> Client { + Client { + connector: DynConnector::new(self.connector), + middleware: self.middleware, + retry_policy: self.retry_policy, + } } -} -pub type BoxFuture = Pin> + Send>>; -pub struct BoxCloneService { - inner: Box< - dyn CloneService> + Send + 'static, - >, + /// Erase the connector and middleware types from the client type signature. + /// + /// This makes the final client type easier to name, at the cost of a marginal increase in + /// runtime performance. See [`DynConnector`] and [`DynMiddleware`] for details. + /// + /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` + /// is equivalent to `Client` with no type arguments. + /// + /// In practice, you'll use this method once you've constructed a client to your liking: + /// + /// ```rust + /// # #[cfg(feature = "https")] + /// # fn not_main() { + /// use smithy_client::{Builder, Client}; + /// struct MyClient { + /// client: smithy_client::Client, + /// } + /// + /// let client = Builder::new() + /// .https() + /// .middleware(tower::layer::util::Identity::new()) + /// .build(); + /// let client = MyClient { client: client.simplify() }; + /// # client.client.check(); + /// # } + pub fn simplify(self) -> DynClient { + self.erase_connector().erase_middleware() + } } -#[derive(Debug, Clone)] -struct Boxed { - inner: S, +/// A Smithy connector that uses dynamic dispatch. +/// +/// This type allows you to pay a small runtime cost to avoid having to name the exact connector +/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to +/// enable dynamic dispatch for every request that goes through the connector, which increases +/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely +/// to matter in all but the highest-performance settings. +#[non_exhaustive] +#[derive(Clone)] +pub struct DynConnector(BoxCloneService, http::Response, BoxError>); + +impl fmt::Debug for DynConnector { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("DynConnector").finish() + } } -impl BoxCloneService { - #[allow(missing_docs)] - pub fn new(inner: S) -> Self +impl DynConnector { + /// Construct a new dynamically-dispatched Smithy middleware. + pub fn new(connector: C) -> Self where - S: Service + Send + 'static + Clone, - S::Future: Send + 'static, + C: bounds::SmithyConnector + Send + 'static, + E: Into, { - let inner = Box::new(Boxed { inner }); - BoxCloneService { inner } - } -} - -impl Clone for BoxCloneService -where - T: 'static, - U: 'static, - E: 'static, -{ - fn clone(&self) -> Self { - Self { - inner: self.clone_box(), - } + Self(BoxCloneService::new(connector.map_err(|e| e.into()))) } } -impl Service for BoxCloneService { - type Response = U; - type Error = E; - type Future = BoxFuture; +impl Service> for DynConnector { + type Response = http::Response; + type Error = BoxError; + type Future = BoxFuture; - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - self.inner.poll_ready(cx) + fn poll_ready( + &mut self, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.0.poll_ready(cx) } - fn call(&mut self, request: T) -> BoxFuture { - self.inner.call(request) + fn call(&mut self, req: http::Request) -> Self::Future { + self.0.call(req) } } -impl fmt::Debug for BoxCloneService { +/// A Smithy middleware that uses dynamic dispatch. +/// +/// This type allows you to pay a small runtime cost to avoid having to name the exact middleware +/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to +/// enable dynamic dispatch for every request that goes through the middleware, which increases +/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely +/// to matter in all but the highest-performance settings. +#[non_exhaustive] +pub struct DynMiddleware( + BoxCloneLayer< + smithy_http_tower::dispatch::DispatchService, + smithy_http::operation::Request, + http::Response, + smithy_http_tower::SendOperationError, + >, +); + +impl fmt::Debug for DynMiddleware { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("BoxCloneService").finish() + fmt.debug_struct("DynMiddleware").finish() } } -impl Service for Boxed -where - S: Service + 'static, - S::Future: Send + 'static, -{ - type Response = S::Response; - type Error = S::Error; - type Future = Pin> + Send>>; - - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - self.inner.poll_ready(cx) +impl DynMiddleware { + /// Construct a new dynamically-dispatched Smithy middleware. + pub fn new + Send + 'static>(middleware: M) -> Self { + Self(BoxCloneLayer::new(middleware)) } +} + +impl Layer> for DynMiddleware { + type Service = BoxCloneService< + smithy_http::operation::Request, + http::Response, + smithy_http_tower::SendOperationError, + >; - fn call(&mut self, request: Request) -> Self::Future { - Box::pin(self.inner.call(request)) + fn layer(&self, inner: smithy_http_tower::dispatch::DispatchService) -> Self::Service { + self.0.layer(inner) } } diff --git a/rust-runtime/smithy-client/src/erase/boxclone.rs b/rust-runtime/smithy-client/src/erase/boxclone.rs new file mode 100644 index 0000000000..d29a652801 --- /dev/null +++ b/rust-runtime/smithy-client/src/erase/boxclone.rs @@ -0,0 +1,151 @@ +// This is an adaptation of tower::util::{BoxLayer, BoxService} that includes Clone and doesn't +// include Sync. + +use std::fmt; +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; +use tower::layer::{layer_fn, Layer}; +use tower::Service; + +pub(super) struct BoxCloneLayer { + boxed: Box>>, +} + +impl BoxCloneLayer { + /// Create a new [`BoxLayer`]. + pub fn new(inner_layer: L) -> Self + where + L: Layer + Send + 'static, + L::Service: Service + Clone + Send + 'static, + >::Future: Send + 'static, + { + let layer = layer_fn(move |inner: In| { + let out = inner_layer.layer(inner); + BoxCloneService::new(out) + }); + + Self { + boxed: Box::new(layer), + } + } +} + +impl Layer for BoxCloneLayer { + type Service = BoxCloneService; + + fn layer(&self, inner: In) -> Self::Service { + self.boxed.layer(inner) + } +} + +impl fmt::Debug for BoxCloneLayer { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("BoxCloneLayer").finish() + } +} + +trait CloneService: Service { + fn clone_box( + &self, + ) -> Box< + dyn CloneService + + Send + + 'static, + >; +} + +impl CloneService for T +where + T: Service + Clone + Send + 'static, +{ + fn clone_box( + &self, + ) -> Box< + dyn CloneService< + Request, + Response = Self::Response, + Error = Self::Error, + Future = Self::Future, + > + + 'static + + Send, + > { + Box::new(self.clone()) + } +} + +pub type BoxFuture = Pin> + Send>>; +pub struct BoxCloneService { + inner: Box< + dyn CloneService> + Send + 'static, + >, +} + +#[derive(Debug, Clone)] +struct Boxed { + inner: S, +} + +impl BoxCloneService { + #[allow(missing_docs)] + pub fn new(inner: S) -> Self + where + S: Service + Send + 'static + Clone, + S::Future: Send + 'static, + { + let inner = Box::new(Boxed { inner }); + BoxCloneService { inner } + } +} + +impl Clone for BoxCloneService +where + T: 'static, + U: 'static, + E: 'static, +{ + fn clone(&self) -> Self { + Self { + inner: self.clone_box(), + } + } +} + +impl Service for BoxCloneService { + type Response = U; + type Error = E; + type Future = BoxFuture; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, request: T) -> BoxFuture { + self.inner.call(request) + } +} + +impl fmt::Debug for BoxCloneService { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("BoxCloneService").finish() + } +} + +impl Service for Boxed +where + S: Service + 'static, + S::Future: Send + 'static, +{ + type Response = S::Response; + type Error = S::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, request: Request) -> Self::Future { + Box::pin(self.inner.call(request)) + } +} diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index 66a31482b2..681d57435b 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -10,14 +10,25 @@ rust_2018_idioms )] +pub mod bounds; +pub mod erase; pub mod retry; +mod builder; +pub use builder::Builder; + #[cfg(feature = "test-util")] pub mod test_connection; #[cfg(feature = "hyper")] mod hyper_impls; +// The types in this module are only used to write the bounds in [`Client::check`]. Customers will +// not need them. But the module and its types must be public so that we can call `check` from +// doc-tests. +#[doc(hidden)] +pub mod static_tests; + /// Type aliases for standard connection types. #[cfg(feature = "hyper")] #[allow(missing_docs)] @@ -46,7 +57,6 @@ use smithy_http_tower::dispatch::DispatchLayer; use smithy_http_tower::parse_response::ParseResponseLayer; use smithy_types::retry::ProvideErrorKind; use std::error::Error; -use std::fmt; use tower::{Layer, Service, ServiceBuilder, ServiceExt}; type BoxError = Box; @@ -72,8 +82,8 @@ type BoxError = Box; /// [`Builder::native_tls`] respectively. #[derive(Debug)] pub struct Client< - Connector = DynConnector, - Middleware = DynMiddleware, + Connector = erase::DynConnector, + Middleware = erase::DynMiddleware, RetryPolicy = retry::Standard, > { connector: Connector, @@ -88,177 +98,6 @@ impl Client { } } -/// A builder that provides more customization options when constructing a [`Client`]. -/// -/// To start, call [`Builder::new`]. Then, chain the method calls to configure the `Builder`. -/// When configured to your liking, call [`Builder::build`]. The individual methods have additional -/// documentation. -#[derive(Clone, Debug)] -pub struct Builder { - connector: C, - middleware: M, - retry_policy: R, -} - -impl Default for Builder<(), ()> { - fn default() -> Self { - Builder { - connector: (), - middleware: (), - retry_policy: retry::Standard::default(), - } - } -} - -impl Builder<(), ()> { - /// Construct a new, unconfigured builder. - /// - /// This builder cannot yet be used, as it does not specify a [connector](Builder::connector) - /// or [middleware](Builder::middleware). It uses the [standard retry - /// mechanism](retry::Standard). - pub fn new() -> Self { - Self::default() - } -} - -impl Builder { - /// Specify the connector for the eventual client to use. - /// - /// The connector dictates how requests are turned into responses. Normally, this would entail - /// sending the request to some kind of remote server, but in certain settings it's useful to - /// be able to use a custom connector instead, such as to mock the network for tests. - /// - /// If you want to use a custom hyper connector, use [`Builder::hyper`]. - /// - /// If you just want to specify a function from request to response instead, use - /// [`Builder::map_connector`]. - pub fn connector(self, connector: C2) -> Builder { - Builder { - connector, - retry_policy: self.retry_policy, - middleware: self.middleware, - } - } - - /// Specify the middleware for the eventual client ot use. - /// - /// The middleware adjusts requests before they are dispatched to the connector. It is - /// responsible for filling in any request parameters that aren't specified by the Smithy - /// protocol definition, such as those used for routing (like the URL), authentication, and - /// authorization. - /// - /// The middleware takes the form of a [`tower::Layer`] that wraps the actual connection for - /// each request. The [`tower::Service`] that the middleware produces must accept requests of - /// the type [`smithy_http::operation::Request`] and return responses of the type - /// [`http::Response`], most likely by modifying the provided request in place, - /// passing it to the inner service, and then ultimately returning the inner service's - /// response. - /// - /// If your requests are already ready to be sent and need no adjustment, you can use - /// [`tower::layer::util::Identity`] as your middleware. - pub fn middleware(self, middleware: M2) -> Builder { - Builder { - connector: self.connector, - retry_policy: self.retry_policy, - middleware, - } - } - - /// Specify the retry policy for the eventual client to use. - /// - /// By default, the Smithy client uses a standard retry policy that works well in most - /// settings. You can use this method to override that policy with a custom one. A new policy - /// instance will be instantiated for each request using [`retry::NewRequestPolicy`]. Each - /// policy instance must implement [`tower::retry::Policy`]. - /// - /// If you just want to modify the policy _configuration_ for the standard retry policy, use - /// [`Builder::set_retry_config`]. - pub fn retry_policy(self, retry_policy: R2) -> Builder { - Builder { - connector: self.connector, - retry_policy, - middleware: self.middleware, - } - } -} - -impl Builder { - /// Set the standard retry policy's configuration. - pub fn set_retry_config(&mut self, config: retry::Config) { - self.retry_policy.with_config(config); - } -} - -impl Builder { - /// Use a connector that directly maps each request to a response. - /// - /// ```rust - /// use smithy_client::Builder; - /// use smithy_http::body::SdkBody; - /// let client = Builder::new() - /// # /* - /// .middleware(..) - /// # */ - /// # .middleware(tower::layer::util::Identity::new()) - /// .map_connector(|req: http::Request| { - /// async move { - /// Ok(http::Response::new(SdkBody::empty())) - /// } - /// }) - /// .build(); - /// # client.check(); - /// ``` - pub fn map_connector(self, map: F) -> Builder, M, R> - where - F: Fn(http::Request) -> FF + Send, - FF: std::future::Future, BoxError>>, - { - self.connector(tower::service_fn(map)) - } -} - -impl Builder { - /// Build a Smithy service [`Client`]. - pub fn build(self) -> Client { - Client { - connector: self.connector, - retry_policy: self.retry_policy, - middleware: self.middleware, - } - } -} - -impl Builder -where - C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, - R: retry::NewRequestPolicy, -{ - /// Build a type-erased Smithy service [`Client`]. - /// - /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` - /// is equivalent to [`Client`] with no type arguments. - /// - /// ```rust - /// # #[cfg(feature = "https")] - /// # fn not_main() { - /// use smithy_client::{Builder, Client}; - /// struct MyClient { - /// client: smithy_client::Client, - /// } - /// - /// let client = Builder::new() - /// .https() - /// .middleware(tower::layer::util::Identity::new()) - /// .build_dyn(); - /// let client = MyClient { client }; - /// # client.client.check(); - /// # } - pub fn build_dyn(self) -> DynClient { - self.build().simplify() - } -} - impl Client where C: bounds::SmithyConnector, @@ -329,447 +168,3 @@ where }; } } - -impl Client -where - C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, - R: retry::NewRequestPolicy, -{ - /// Erase the middleware type from the client type signature. - /// - /// This makes the final client type easier to name, at the cost of a marginal increase in - /// runtime performance. See [`DynMiddleware`] for details. - /// - /// In practice, you'll use this method once you've constructed a client to your liking: - /// - /// ```rust - /// # #[cfg(feature = "https")] - /// # fn not_main() { - /// use smithy_client::{Builder, Client}; - /// struct MyClient { - /// client: Client, - /// } - /// - /// let client = Builder::new() - /// .https() - /// .middleware(tower::layer::util::Identity::new()) - /// .build(); - /// let client = MyClient { client: client.erase_middleware() }; - /// # client.client.check(); - /// # } - pub fn erase_middleware(self) -> Client, R> { - Client { - connector: self.connector, - middleware: DynMiddleware::new(self.middleware), - retry_policy: self.retry_policy, - } - } -} - -impl Client -where - C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, - R: retry::NewRequestPolicy, -{ - /// Erase the connector type from the client type signature. - /// - /// This makes the final client type easier to name, at the cost of a marginal increase in - /// runtime performance. See [`DynConnector`] for details. - /// - /// In practice, you'll use this method once you've constructed a client to your liking: - /// - /// ```rust - /// # #[cfg(feature = "https")] - /// # fn not_main() { - /// # type MyMiddleware = smithy_client::DynMiddleware; - /// use smithy_client::{Builder, Client}; - /// struct MyClient { - /// client: Client, - /// } - /// - /// let client = Builder::new() - /// .https() - /// .middleware(tower::layer::util::Identity::new()) - /// .build(); - /// let client = MyClient { client: client.erase_connector() }; - /// # client.client.check(); - /// # } - pub fn erase_connector(self) -> Client { - Client { - connector: DynConnector::new(self.connector), - middleware: self.middleware, - retry_policy: self.retry_policy, - } - } - - /// Erase the connector and middleware types from the client type signature. - /// - /// This makes the final client type easier to name, at the cost of a marginal increase in - /// runtime performance. See [`DynConnector`] and [`DynMiddleware`] for details. - /// - /// Note that if you're using the standard retry mechanism, [`retry::Standard`], `DynClient` - /// is equivalent to `Client` with no type arguments. - /// - /// In practice, you'll use this method once you've constructed a client to your liking: - /// - /// ```rust - /// # #[cfg(feature = "https")] - /// # fn not_main() { - /// use smithy_client::{Builder, Client}; - /// struct MyClient { - /// client: smithy_client::Client, - /// } - /// - /// let client = Builder::new() - /// .https() - /// .middleware(tower::layer::util::Identity::new()) - /// .build(); - /// let client = MyClient { client: client.simplify() }; - /// # client.client.check(); - /// # } - pub fn simplify(self) -> DynClient { - self.erase_connector().erase_middleware() - } -} - -// These types are technically public in that they're reachable from the public trait impls on -// DynMiddleware, but no-one should ever look at them or use them. -#[doc(hidden)] -pub mod erase; - -/// A [`Client`] whose connector and middleware types have been erased. -/// -/// Mainly useful if you need to name `R` in a type-erased client. If you do not, you can instead -/// just use `Client` with no type parameters, which ends up being the same type. -pub type DynClient = Client, R>; - -/// A Smithy connector that uses dynamic dispatch. -/// -/// This type allows you to pay a small runtime cost to avoid having to name the exact connector -/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to -/// enable dynamic dispatch for every request that goes through the connector, which increases -/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely -/// to matter in all but the highest-performance settings. -#[non_exhaustive] -#[derive(Clone)] -pub struct DynConnector( - erase::BoxCloneService, http::Response, BoxError>, -); - -impl fmt::Debug for DynConnector { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("DynConnector").finish() - } -} - -impl DynConnector { - /// Construct a new dynamically-dispatched Smithy middleware. - pub fn new(connector: C) -> Self - where - C: bounds::SmithyConnector + Send + 'static, - E: Into, - { - Self(erase::BoxCloneService::new(connector.map_err(|e| e.into()))) - } -} - -impl Service> for DynConnector { - type Response = http::Response; - type Error = BoxError; - type Future = erase::BoxFuture; - - fn poll_ready( - &mut self, - cx: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - self.0.poll_ready(cx) - } - - fn call(&mut self, req: http::Request) -> Self::Future { - self.0.call(req) - } -} - -/// A Smithy middleware that uses dynamic dispatch. -/// -/// This type allows you to pay a small runtime cost to avoid having to name the exact middleware -/// you're using anywhere you want to hold a [`Client`]. Specifically, this will use `Box` to -/// enable dynamic dispatch for every request that goes through the middleware, which increases -/// memory pressure and suffers an additional vtable indirection for each request, but is unlikely -/// to matter in all but the highest-performance settings. -#[non_exhaustive] -pub struct DynMiddleware( - erase::BoxCloneLayer< - smithy_http_tower::dispatch::DispatchService, - smithy_http::operation::Request, - http::Response, - smithy_http_tower::SendOperationError, - >, -); - -impl fmt::Debug for DynMiddleware { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.debug_struct("DynMiddleware").finish() - } -} - -impl DynMiddleware { - /// Construct a new dynamically-dispatched Smithy middleware. - pub fn new + Send + 'static>(middleware: M) -> Self { - Self(erase::BoxCloneLayer::new(middleware)) - } -} - -impl Layer> for DynMiddleware { - type Service = erase::BoxCloneService< - smithy_http::operation::Request, - http::Response, - smithy_http_tower::SendOperationError, - >; - - fn layer(&self, inner: smithy_http_tower::dispatch::DispatchService) -> Self::Service { - self.0.layer(inner) - } -} - -/// This module holds convenient short-hands for the otherwise fairly extensive trait bounds -/// required for `call` and friends. -/// -/// The short-hands will one day be true [trait aliases], but for now they are traits with blanket -/// implementations. Also, due to [compiler limitations], the bounds repeat a nubmer of associated -/// types with bounds so that those bounds [do not need to be repeated] at the call site. It's a -/// bit of a mess to define, but _should_ be invisible to callers. -/// -/// [trait aliases]: https://rust-lang.github.io/rfcs/1733-trait-alias.html -/// [compiler limitations]: https://github.com/rust-lang/rust/issues/20671 -/// [do not need to be repeated]: https://github.com/rust-lang/rust/issues/20671#issuecomment-529752828 -pub mod bounds { - use super::*; - - /// A service that has parsed a raw Smithy response. - pub type Parsed = - smithy_http_tower::parse_response::ParseResponseService; - - /// A low-level Smithy connector that maps from [`http::Request`] to [`http::Response`]. - /// - /// This trait has a blanket implementation for all compatible types, and should never need to - /// be implemented. - pub trait SmithyConnector: - Service< - http::Request, - Response = http::Response, - Error = ::Error, - Future = ::Future, - > + Send - + Clone - + 'static - { - /// Forwarding type to `::Error` for bound inference. - /// - /// See module-level docs for details. - type Error: Into + Send + Sync + 'static; - - /// Forwarding type to `::Future` for bound inference. - /// - /// See module-level docs for details. - type Future: Send + 'static; - } - - impl SmithyConnector for T - where - T: Service, Response = http::Response> - + Send - + Clone - + 'static, - T::Error: Into + Send + Sync + 'static, - T::Future: Send + 'static, - { - type Error = T::Error; - type Future = T::Future; - } - - /// A Smithy middleware service that adjusts [`smithy_http::operation::Request`]s. - /// - /// This trait has a blanket implementation for all compatible types, and should never need to - /// be implemented. - pub trait SmithyMiddlewareService: - Service< - smithy_http::operation::Request, - Response = http::Response, - Error = smithy_http_tower::SendOperationError, - Future = ::Future, - > - { - /// Forwarding type to `::Future` for bound inference. - /// - /// See module-level docs for details. - type Future: Send + 'static; - } - - impl SmithyMiddlewareService for T - where - T: Service< - smithy_http::operation::Request, - Response = http::Response, - Error = smithy_http_tower::SendOperationError, - >, - T::Future: Send + 'static, - { - type Future = T::Future; - } - - /// A Smithy middleware layer (i.e., factory). - /// - /// This trait has a blanket implementation for all compatible types, and should never need to - /// be implemented. - pub trait SmithyMiddleware: - Layer< - smithy_http_tower::dispatch::DispatchService, - Service = >::Service, - > - { - /// Forwarding type to `::Service` for bound inference. - /// - /// See module-level docs for details. - type Service: SmithyMiddlewareService + Send + Clone + 'static; - } - - impl SmithyMiddleware for T - where - T: Layer>, - T::Service: SmithyMiddlewareService + Send + Clone + 'static, - { - type Service = T::Service; - } - - /// A Smithy retry policy. - /// - /// This trait has a blanket implementation for all compatible types, and should never need to - /// be implemented. - pub trait SmithyRetryPolicy: - tower::retry::Policy, SdkSuccess, SdkError> + Clone - { - /// Forwarding type to `O` for bound inference. - /// - /// See module-level docs for details. - type O: ParseHttpResponse> - + Send - + Sync - + Clone - + 'static; - /// Forwarding type to `E` for bound inference. - /// - /// See module-level docs for details. - type E: Error + ProvideErrorKind; - - /// Forwarding type to `Retry` for bound inference. - /// - /// See module-level docs for details. - type Retry: ClassifyResponse, SdkError>; - } - - impl SmithyRetryPolicy for R - where - R: tower::retry::Policy, SdkSuccess, SdkError> + Clone, - O: ParseHttpResponse> + Send + Sync + Clone + 'static, - E: Error + ProvideErrorKind, - Retry: ClassifyResponse, SdkError>, - { - type O = O; - type E = E; - type Retry = Retry; - } -} - -/// This module provides types useful for static tests. -/// -/// These are only used to write the bounds in [`Client::check`]. Customers will not need them. -/// But the module and its types must be public so that we can call `check` from doc-tests. -#[doc(hidden)] -#[allow(missing_docs, missing_debug_implementations)] -pub mod static_tests { - use super::*; - - #[derive(Debug)] - #[non_exhaustive] - pub struct TestOperationError; - impl std::fmt::Display for TestOperationError { - fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - unreachable!("only used for static tests") - } - } - impl Error for TestOperationError {} - impl ProvideErrorKind for TestOperationError { - fn retryable_error_kind(&self) -> Option { - unreachable!("only used for static tests") - } - - fn code(&self) -> Option<&str> { - unreachable!("only used for static tests") - } - } - #[derive(Clone)] - #[non_exhaustive] - pub struct TestOperation; - impl ParseHttpResponse for TestOperation { - type Output = Result<(), TestOperationError>; - - fn parse_unloaded(&self, _: &mut http::Response) -> Option { - unreachable!("only used for static tests") - } - - fn parse_loaded(&self, _response: &http::Response) -> Self::Output { - unreachable!("only used for static tests") - } - } - pub type ValidTestOperation = Operation; - - // Statically check that a standard retry can actually be used to build a Client. - #[allow(dead_code)] - #[cfg(test)] - fn sanity_retry() { - Builder::new() - .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) - .build() - .check(); - } - - // Statically check that a hyper client can actually be used to build a Client. - #[allow(dead_code)] - #[cfg(all(test, feature = "hyper"))] - fn sanity_hyper(hc: hyper::Client) - where - C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, - { - Builder::new() - .middleware(tower::layer::util::Identity::new()) - .hyper(hc) - .build() - .check(); - } - - // Statically check that a type-erased middleware client is actually a valid Client. - #[allow(dead_code)] - fn sanity_erase_middleware() { - Builder::new() - .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) - .build() - .erase_middleware() - .check(); - } - - // Statically check that a type-erased connector client is actually a valid Client. - #[allow(dead_code)] - fn sanity_erase_connector() { - Builder::new() - .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) - .build() - .erase_connector() - .check(); - } -} diff --git a/rust-runtime/smithy-client/src/static_tests.rs b/rust-runtime/smithy-client/src/static_tests.rs new file mode 100644 index 0000000000..e520be89b7 --- /dev/null +++ b/rust-runtime/smithy-client/src/static_tests.rs @@ -0,0 +1,85 @@ +//! This module provides types useful for static tests. +#![allow(missing_docs, missing_debug_implementations)] + +use crate::*; + +#[derive(Debug)] +#[non_exhaustive] +pub struct TestOperationError; +impl std::fmt::Display for TestOperationError { + fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + unreachable!("only used for static tests") + } +} +impl Error for TestOperationError {} +impl ProvideErrorKind for TestOperationError { + fn retryable_error_kind(&self) -> Option { + unreachable!("only used for static tests") + } + + fn code(&self) -> Option<&str> { + unreachable!("only used for static tests") + } +} +#[derive(Clone)] +#[non_exhaustive] +pub struct TestOperation; +impl ParseHttpResponse for TestOperation { + type Output = Result<(), TestOperationError>; + + fn parse_unloaded(&self, _: &mut http::Response) -> Option { + unreachable!("only used for static tests") + } + + fn parse_loaded(&self, _response: &http::Response) -> Self::Output { + unreachable!("only used for static tests") + } +} +pub type ValidTestOperation = Operation; + +// Statically check that a standard retry can actually be used to build a Client. +#[allow(dead_code)] +#[cfg(test)] +fn sanity_retry() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .check(); +} + +// Statically check that a hyper client can actually be used to build a Client. +#[allow(dead_code)] +#[cfg(all(test, feature = "hyper"))] +fn sanity_hyper(hc: hyper::Client) +where + C: hyper::client::connect::Connect + Clone + Send + Sync + 'static, +{ + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .hyper(hc) + .build() + .check(); +} + +// Statically check that a type-erased middleware client is actually a valid Client. +#[allow(dead_code)] +fn sanity_erase_middleware() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .erase_middleware() + .check(); +} + +// Statically check that a type-erased connector client is actually a valid Client. +#[allow(dead_code)] +fn sanity_erase_connector() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .map_connector(|_| async { unreachable!() }) + .build() + .erase_connector() + .check(); +} From 48c9b77fb7baaa6b6d9288f5491cc778fd511487 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 14:18:50 -0700 Subject: [PATCH 14/36] Make Builder a little nicer to use --- rust-runtime/smithy-client/src/builder.rs | 116 +++++++++++------- rust-runtime/smithy-client/src/hyper_impls.rs | 6 +- .../smithy-client/src/static_tests.rs | 6 +- 3 files changed, 79 insertions(+), 49 deletions(-) diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs index 1f0ca98b91..afd7a7212f 100644 --- a/rust-runtime/smithy-client/src/builder.rs +++ b/rust-runtime/smithy-client/src/builder.rs @@ -6,27 +6,31 @@ use smithy_http::body::SdkBody; /// To start, call [`Builder::new`]. Then, chain the method calls to configure the `Builder`. /// When configured to your liking, call [`Builder::build`]. The individual methods have additional /// documentation. -#[derive(Clone, Debug)] -pub struct Builder { +#[derive(Clone, Debug, Default)] +pub struct Builder { connector: C, middleware: M, retry_policy: R, } -impl Default for Builder<(), ()> { - fn default() -> Self { - Builder { - connector: (), - middleware: (), - retry_policy: retry::Standard::default(), - } - } -} - -impl Builder<(), ()> { - /// Construct a new, unconfigured builder. +// It'd be nice to include R where R: Default here, but then the caller ends up always having to +// specify R explicitly since type parameter defaults (like the one for R) aren't picked up when R +// cannot be inferred. This is, arguably, a compiler bug/missing language feature, but is +// complicated: https://github.com/rust-lang/rust/issues/27336. +// +// For the time being, we stick with just for ::new. Those can usually be inferred since we +// only implement .constructor and .middleware when C and M are () respectively. Users who really +// need a builder for a custom R can use ::default instead. +impl Builder +where + C: Default, + M: Default, +{ + /// Construct a new builder. /// - /// This builder cannot yet be used, as it does not specify a [connector](Builder::connector) + /// This will + /// + /// You will likely want to , as it does not specify a [connector](Builder::connector) /// or [middleware](Builder::middleware). It uses the [standard retry /// mechanism](retry::Standard). pub fn new() -> Self { @@ -34,7 +38,7 @@ impl Builder<(), ()> { } } -impl Builder { +impl Builder<(), M, R> { /// Specify the connector for the eventual client to use. /// /// The connector dictates how requests are turned into responses. Normally, this would entail @@ -45,7 +49,7 @@ impl Builder { /// /// If you just want to specify a function from request to response instead, use /// [`Builder::map_connector`]. - pub fn connector(self, connector: C2) -> Builder { + pub fn connector(self, connector: C) -> Builder { Builder { connector, retry_policy: self.retry_policy, @@ -53,6 +57,34 @@ impl Builder { } } + /// Use a function that directly maps each request to a response as a connector. + /// + /// ```rust + /// use smithy_client::Builder; + /// use smithy_http::body::SdkBody; + /// let client = Builder::new() + /// # /* + /// .middleware(..) + /// # */ + /// # .middleware(tower::layer::util::Identity::new()) + /// .connector_fn(|req: http::Request| { + /// async move { + /// Ok(http::Response::new(SdkBody::empty())) + /// } + /// }) + /// .build(); + /// # client.check(); + /// ``` + pub fn connector_fn(self, map: F) -> Builder, M, R> + where + F: Fn(http::Request) -> FF + Send, + FF: std::future::Future, BoxError>>, + { + self.connector(tower::service_fn(map)) + } +} + +impl Builder { /// Specify the middleware for the eventual client ot use. /// /// The middleware adjusts requests before they are dispatched to the connector. It is @@ -69,14 +101,16 @@ impl Builder { /// /// If your requests are already ready to be sent and need no adjustment, you can use /// [`tower::layer::util::Identity`] as your middleware. - pub fn middleware(self, middleware: M2) -> Builder { + pub fn middleware(self, middleware: M) -> Builder { Builder { connector: self.connector, retry_policy: self.retry_policy, middleware, } } +} +impl Builder { /// Specify the retry policy for the eventual client to use. /// /// By default, the Smithy client uses a standard retry policy that works well in most @@ -86,7 +120,7 @@ impl Builder { /// /// If you just want to modify the policy _configuration_ for the standard retry policy, use /// [`Builder::set_retry_config`]. - pub fn retry_policy(self, retry_policy: R2) -> Builder { + pub fn retry_policy(self, retry_policy: R) -> Builder { Builder { connector: self.connector, retry_policy, @@ -103,34 +137,30 @@ impl Builder { } impl Builder { - /// Use a connector that directly maps each request to a response. - /// - /// ```rust - /// use smithy_client::Builder; - /// use smithy_http::body::SdkBody; - /// let client = Builder::new() - /// # /* - /// .middleware(..) - /// # */ - /// # .middleware(tower::layer::util::Identity::new()) - /// .map_connector(|req: http::Request| { - /// async move { - /// Ok(http::Response::new(SdkBody::empty())) - /// } - /// }) - /// .build(); - /// # client.check(); - /// ``` - pub fn map_connector(self, map: F) -> Builder, M, R> + /// Use a connector that wraps the current connector. + pub fn map_connector(self, map: F) -> Builder where - F: Fn(http::Request) -> FF + Send, - FF: std::future::Future, BoxError>>, + F: FnOnce(C) -> C2, { - self.connector(tower::service_fn(map)) + Builder { + connector: map(self.connector), + middleware: self.middleware, + retry_policy: self.retry_policy, + } + } + + /// Use a middleware that wraps the current middleware. + pub fn map_middleware(self, map: F) -> Builder + where + F: FnOnce(M) -> M2, + { + Builder { + connector: self.connector, + middleware: map(self.middleware), + retry_policy: self.retry_policy, + } } -} -impl Builder { /// Build a Smithy service [`Client`]. pub fn build(self) -> Client { Client { diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index 95dc76ec75..9974454e66 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -37,7 +37,7 @@ impl From> for HyperAdapter { } } -impl Builder { +impl Builder<(), M, R> { /// Connect to the service using the provided `hyper` client. pub fn hyper(self, connector: hyper::Client) -> Builder, M, R> where @@ -57,7 +57,7 @@ impl crate::Client Builder { +impl Builder<(), M, R> { /// Connect to the service over HTTPS using Rustls. pub fn rustls( self, @@ -80,7 +80,7 @@ impl Builder { } } #[cfg(feature = "native-tls")] -impl Builder { +impl Builder<(), M, R> { /// Connect to the service over HTTPS using the native TLS library on your platform. pub fn native_tls( self, diff --git a/rust-runtime/smithy-client/src/static_tests.rs b/rust-runtime/smithy-client/src/static_tests.rs index e520be89b7..35bdda801c 100644 --- a/rust-runtime/smithy-client/src/static_tests.rs +++ b/rust-runtime/smithy-client/src/static_tests.rs @@ -43,7 +43,7 @@ pub type ValidTestOperation = Operation; fn sanity_retry() { Builder::new() .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) + .connector_fn(|_| async { unreachable!() }) .build() .check(); } @@ -67,7 +67,7 @@ where fn sanity_erase_middleware() { Builder::new() .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) + .connector_fn(|_| async { unreachable!() }) .build() .erase_middleware() .check(); @@ -78,7 +78,7 @@ fn sanity_erase_middleware() { fn sanity_erase_connector() { Builder::new() .middleware(tower::layer::util::Identity::new()) - .map_connector(|_| async { unreachable!() }) + .connector_fn(|_| async { unreachable!() }) .build() .erase_connector() .check(); From ccdaaf48685f9e43b26baac6925159c043239870 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 14:49:55 -0700 Subject: [PATCH 15/36] Make aws_hyper simply wrap smithy_client --- aws/rust-runtime/aws-hyper/src/conn.rs | 220 ------------------ aws/rust-runtime/aws-hyper/src/lib.rs | 124 +++------- .../aws-hyper/src/test_connection.rs | 142 ----------- aws/rust-runtime/aws-hyper/tests/e2e_test.rs | 2 +- rust-runtime/smithy-client/src/bounds.rs | 11 +- rust-runtime/smithy-client/src/builder.rs | 2 +- rust-runtime/smithy-client/src/erase.rs | 6 +- .../smithy-client/src/erase/boxclone.rs | 19 +- rust-runtime/smithy-client/src/hyper_impls.rs | 28 ++- rust-runtime/smithy-client/src/lib.rs | 21 ++ .../smithy-client/src/static_tests.rs | 28 +++ 11 files changed, 130 insertions(+), 473 deletions(-) delete mode 100644 aws/rust-runtime/aws-hyper/src/conn.rs delete mode 100644 aws/rust-runtime/aws-hyper/src/test_connection.rs diff --git a/aws/rust-runtime/aws-hyper/src/conn.rs b/aws/rust-runtime/aws-hyper/src/conn.rs deleted file mode 100644 index d32fd4efc1..0000000000 --- a/aws/rust-runtime/aws-hyper/src/conn.rs +++ /dev/null @@ -1,220 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0. - */ - -use crate::BoxError; -use http::Request; -use hyper::client::ResponseFuture; -use hyper::Response; -use smithy_http::body::SdkBody; -use std::fmt; -use std::future::Future; -use std::pin::Pin; -use std::task::{Context, Poll}; -use tower::Service; - -#[derive(Clone, Debug)] -pub struct Standard(Connector); - -impl Standard { - /// An https connection - /// - /// If the `rustls` feature is enabled, this will use `rustls`. - /// If the ONLY the `native-tls` feature is enabled, this will use `native-tls`. - /// If both features are enabled, this will use `rustls` - #[cfg(any(feature = "native-tls", feature = "rustls"))] - pub fn https() -> Self { - #[cfg(feature = "rustls")] - { - Self::rustls() - } - - // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled - #[cfg(not(feature = "rustls"))] - { - Self::native_tls() - } - } - - #[cfg(feature = "rustls")] - pub fn rustls() -> Self { - let https = hyper_rustls::HttpsConnector::with_native_roots(); - let client = hyper::Client::builder().build::<_, SdkBody>(https); - Self(Connector::RustlsHttps(client)) - } - - #[cfg(feature = "native-tls")] - pub fn native_tls() -> Self { - let https = hyper_tls::HttpsConnector::new(); - let client = hyper::Client::builder().build::<_, SdkBody>(https); - Self(Connector::NativeHttps(client)) - } - - /// A connection based on the provided `impl HttpService` - /// - /// Generally, [`Standard::https()`](Standard::https) should be used. This constructor is intended to support - /// using things like [`TestConnection`](crate::test_connection::TestConnection) or alternative - /// http implementations. - pub fn new(connector: impl HttpService + 'static) -> Self { - Self(Connector::Dyn(Box::new(connector))) - } -} - -/// An Http connection type for most use cases -/// -/// This supports three options: -/// 1. HTTPS -/// 2. A `TestConnection` -/// 3. Any implementation of the `HttpService` trait -/// -/// This is designed to be used with [`aws_hyper::Client`](crate::Client) as a connector. -#[derive(Clone)] -enum Connector { - /// An Https Connection - /// - /// This is the correct connection for use cases talking to real AWS services. - #[cfg(feature = "native-tls")] - NativeHttps(hyper::Client, SdkBody>), - - /// An Https Connection - /// - /// This is the correct connection for use cases talking to real AWS services. - #[cfg(feature = "rustls")] - RustlsHttps(hyper::Client, SdkBody>), - - /// A generic escape hatch - /// - /// This enables using any implementation of the HttpService trait. This allows using a totally - /// separate HTTP stack or your own custom `TestConnection`. - Dyn(Box), -} - -impl fmt::Debug for Connector { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - #[cfg(feature = "native-tls")] - Connector::NativeHttps(tls) => { - f.debug_tuple("Connector::NativeHttps").field(tls).finish() - } - #[cfg(feature = "rustls")] - Connector::RustlsHttps(tls) => { - f.debug_tuple("Connector::RustlsHttps").field(tls).finish() - } - Connector::Dyn(_) => f.debug_tuple("Connector::Dyn").finish(), - } - } -} - -impl Clone for Box { - fn clone(&self) -> Self { - self.clone_box() - } -} - -pub trait HttpService: Send + Sync { - /// Return whether this service is ready to accept a request - /// - /// See [`Service::poll_ready`](tower::Service::poll_ready) - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll>; - - /// Call this service and return a response - /// - /// See [`Service::call`](tower::Service::call) - fn call( - &mut self, - req: http::Request, - ) -> Pin, BoxError>> + Send>>; - - /// Return a Boxed-clone of this service - /// - /// `aws_hyper::Client` will clone the inner service for each request so this should be a cheap - /// clone operation. - fn clone_box(&self) -> Box; -} - -/// Reverse implementation: If you have a correctly shaped tower service, it _is_ an `HttpService` -/// -/// This is to facilitate ease of use for people using `Standard::Dyn` -impl HttpService for S -where - S: Service, Response = http::Response> - + Send - + Sync - + Clone - + 'static, - S::Error: Into + Send + Sync + 'static, - S::Future: Send + 'static, -{ - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - Service::poll_ready(self, cx).map_err(|err| err.into()) - } - - fn call( - &mut self, - req: Request, - ) -> Pin, BoxError>> + Send>> { - let fut = Service::call(self, req); - let fut = async move { - fut.await - .map(|res| res.map(SdkBody::from)) - .map_err(|err| err.into()) - }; - Box::pin(fut) - } - - fn clone_box(&self) -> Box { - Box::new(self.clone()) - } -} - -impl tower::Service> for Standard { - type Response = http::Response; - type Error = BoxError; - type Future = StandardFuture; - - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - match &mut self.0 { - #[cfg(feature = "native-tls")] - Connector::NativeHttps(https) => { - Service::poll_ready(https, cx).map_err(|err| err.into()) - } - #[cfg(feature = "rustls")] - Connector::RustlsHttps(https) => { - Service::poll_ready(https, cx).map_err(|err| err.into()) - } - Connector::Dyn(conn) => conn.poll_ready(cx), - } - } - - fn call(&mut self, req: http::Request) -> Self::Future { - match &mut self.0 { - #[cfg(feature = "native-tls")] - Connector::NativeHttps(https) => StandardFuture::Https(Service::call(https, req)), - #[cfg(feature = "rustls")] - Connector::RustlsHttps(https) => StandardFuture::Https(Service::call(https, req)), - Connector::Dyn(conn) => StandardFuture::Dyn(conn.call(req)), - } - } -} - -/// Future returned by `Standard` when used as a tower::Service -#[pin_project::pin_project(project = FutProj)] -pub enum StandardFuture { - Https(#[pin] ResponseFuture), - Dyn(#[pin] Pin, BoxError>> + Send>>), -} - -impl Future for StandardFuture { - type Output = Result, BoxError>; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - match self.project() { - FutProj::Https(fut) => fut - .poll(cx) - .map(|resp| resp.map(|res| res.map(SdkBody::from))) - .map_err(|err| err.into()), - FutProj::Dyn(dyn_fut) => dyn_fut.poll(cx), - } - } -} diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index 1d343aee74..fdde50e6f6 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -3,41 +3,30 @@ * SPDX-License-Identifier: Apache-2.0. */ -pub mod conn; -#[cfg(feature = "test-util")] -pub mod test_connection; - pub use smithy_client::retry::Config as RetryConfig; -use crate::conn::Standard; use aws_endpoint::AwsEndpointStage; use aws_http::user_agent::UserAgentStage; use aws_sig_auth::middleware::SigV4SigningStage; use aws_sig_auth::signer::SigV4Signer; -use smithy_http::body::SdkBody; -use smithy_http::operation::Operation; -use smithy_http::response::ParseHttpResponse; pub use smithy_http::result::{SdkError, SdkSuccess}; -use smithy_http::retry::ClassifyResponse; use smithy_http_tower::map_request::MapRequestLayer; -use smithy_types::retry::ProvideErrorKind; -use std::error::Error; use std::fmt::Debug; use tower::layer::util::Stack; -use tower::{Service, ServiceBuilder}; +use tower::ServiceBuilder; -type BoxError = Box; -pub type StandardClient = Client; +pub type StandardClient = smithy_client::Client; -type AwsMiddleware = Stack< +type AwsMiddlewareStack = Stack< MapRequestLayer, Stack, MapRequestLayer>, >; -#[derive(Debug)] -struct AwsMiddlewareLayer; -impl tower::Layer for AwsMiddlewareLayer { - type Service = >::Service; +#[derive(Debug, Default)] +#[non_exhaustive] +pub struct AwsMiddleware; +impl tower::Layer for AwsMiddleware { + type Service = >::Service; fn layer(&self, inner: S) -> Self::Service { let signer = MapRequestLayer::for_mapper(SigV4SigningStage::new(SigV4Signer::new())); @@ -71,94 +60,47 @@ impl tower::Layer for AwsMiddlewareLayer { /// S::Error: Into + Send + Sync + 'static, /// S::Future: Send + 'static, /// ``` -#[derive(Debug)] -pub struct Client { - inner: smithy_client::Client, -} - -impl Client { - /// Construct a new `Client` with a custom connector - pub fn new(connector: S) -> Self { - Client { - inner: smithy_client::Builder::new() - .connector(connector) - .middleware(AwsMiddlewareLayer) - .build(), - } - } - - pub fn with_retry_config(mut self, retry_config: RetryConfig) -> Self { - self.inner.set_retry_config(retry_config); - self - } -} - -impl Client { - /// Construct an `https` based client - #[cfg(any(feature = "native-tls", feature = "rustls"))] - pub fn https() -> StandardClient { - Client { - inner: smithy_client::Builder::new() - .connector(Standard::https()) - .middleware(AwsMiddlewareLayer) - .build(), - } - } -} +#[doc(inline)] +pub type Client = smithy_client::Client; -impl Client -where - S: Service, Response = http::Response> + Send + Clone + 'static, - S::Error: Into + Send + Sync + 'static, - S::Future: Send + 'static, -{ - /// Dispatch this request to the network - /// - /// For ergonomics, this does not include the raw response for successful responses. To - /// access the raw response use `call_raw`. - pub async fn call(&self, input: Operation) -> Result> - where - O: ParseHttpResponse> + Send + Sync + Clone + 'static, - E: Error + ProvideErrorKind, - Retry: ClassifyResponse, SdkError>, - { - self.call_raw(input).await.map(|res| res.parsed) - } +#[doc(inline)] +pub type Builder = smithy_client::Builder; - /// Dispatch this request to the network - /// - /// The returned result contains the raw HTTP response which can be useful for debugging or implementing - /// unsupported features. - pub async fn call_raw( - &self, - input: Operation, - ) -> Result, SdkError> - where - O: ParseHttpResponse> + Send + Sync + Clone + 'static, - E: Error + ProvideErrorKind, - Retry: ClassifyResponse, SdkError>, - { - self.inner.call_raw(input).await - } +/// Construct an `https` based client +/// +/// If the `rustls` feature is enabled, this will use `rustls`. +/// If the ONLY the `native-tls` feature is enabled, this will use `native-tls`. +/// If both features are enabled, this will use `rustls` +#[cfg(any(feature = "native-tls", feature = "rustls"))] +pub fn https() -> StandardClient { + #[cfg(feature = "rustls")] + let with_https = |b: Builder<_>| b.rustls(); + // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled + #[cfg(not(feature = "rustls"))] + let with_https = |b: Builder<_>| b.native_tls(); + + with_https(smithy_client::Builder::new()) + .build() + .erase_connector() } -#[cfg(test)] -mod tests { - +mod static_tests { #[cfg(any(feature = "rustls", feature = "native-tls"))] - #[test] + #[allow(dead_code)] fn construct_default_client() { let c = crate::Client::https(); fn is_send_sync(_c: T) {} is_send_sync(c); } +} +#[cfg(test)] +mod tests { #[cfg(any(feature = "rustls", feature = "native-tls"))] #[test] fn client_debug_includes_retry_info() { let client = crate::Client::https(); let s = format!("{:?}", client); - assert!(s.contains("RetryConfig")); assert!(s.contains("quota_available")); } } diff --git a/aws/rust-runtime/aws-hyper/src/test_connection.rs b/aws/rust-runtime/aws-hyper/src/test_connection.rs deleted file mode 100644 index a2749fb647..0000000000 --- a/aws/rust-runtime/aws-hyper/src/test_connection.rs +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0. - */ - -use http::header::{HeaderName, CONTENT_TYPE}; -use http::Request; -use protocol_test_helpers::{assert_ok, validate_body, MediaType}; -use smithy_http::body::SdkBody; -use std::future::Ready; -use std::ops::Deref; -use std::sync::{Arc, Mutex}; -use std::task::{Context, Poll}; -use tower::BoxError; - -type ConnectVec = Vec<(http::Request, http::Response)>; - -pub struct ValidateRequest { - pub expected: http::Request, - pub actual: http::Request, -} - -impl ValidateRequest { - pub fn assert_matches(&self, ignore_headers: Vec) { - let (actual, expected) = (&self.actual, &self.expected); - for (name, value) in expected.headers() { - if !ignore_headers.contains(name) { - let actual_header = actual - .headers() - .get(name) - .unwrap_or_else(|| panic!("Header {:?} missing", name)); - assert_eq!(actual_header, value, "Header mismatch for {:?}", name); - } - } - let actual_str = std::str::from_utf8(actual.body().bytes().unwrap_or(&[])); - let expected_str = std::str::from_utf8(expected.body().bytes().unwrap_or(&[])); - let media_type = if actual - .headers() - .get(CONTENT_TYPE) - .map(|v| v.to_str().unwrap().contains("json")) - .unwrap_or(false) - { - MediaType::Json - } else { - MediaType::Other("unknown".to_string()) - }; - match (actual_str, expected_str) { - (Ok(actual), Ok(expected)) => assert_ok(validate_body(actual, expected, media_type)), - _ => assert_eq!(actual.body().bytes(), expected.body().bytes()), - }; - assert_eq!(actual.uri(), expected.uri()); - } -} - -/// TestConnection for use with a [`aws_hyper::Client`](crate::Client) -/// -/// A basic test connection. It will: -/// - Response to requests with a preloaded series of responses -/// - Record requests for future examination -/// -/// The generic parameter `B` is the type of the response body. -/// For more complex use cases, see [Tower Test](https://docs.rs/tower-test/0.4.0/tower_test/) -/// Usage example: -/// ```rust -/// use aws_hyper::test_connection::TestConnection; -/// use smithy_http::body::SdkBody; -/// let events = vec![( -/// http::Request::new(SdkBody::from("request body")), -/// http::Response::builder() -/// .status(200) -/// .body("response body") -/// .unwrap(), -/// )]; -/// let conn = TestConnection::new(events); -/// let client = aws_hyper::Client::new(conn); -/// ``` -pub struct TestConnection { - data: Arc>>, - requests: Arc>>, -} - -// Need a clone impl that ignores `B` -impl Clone for TestConnection { - fn clone(&self) -> Self { - TestConnection { - data: self.data.clone(), - requests: self.requests.clone(), - } - } -} - -impl TestConnection { - pub fn new(mut data: ConnectVec) -> Self { - data.reverse(); - TestConnection { - data: Arc::new(Mutex::new(data)), - requests: Default::default(), - } - } - - pub fn requests(&self) -> impl Deref> + '_ { - self.requests.lock().unwrap() - } -} - -impl> tower::Service> for TestConnection { - type Response = http::Response; - type Error = BoxError; - type Future = Ready>; - - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, actual: Request) -> Self::Future { - // todo: validate request - if let Some((expected, resp)) = self.data.lock().unwrap().pop() { - self.requests - .lock() - .unwrap() - .push(ValidateRequest { expected, actual }); - std::future::ready(Ok(resp.map(|body| SdkBody::from(body.into())))) - } else { - std::future::ready(Err("No more data".into())) - } - } -} - -#[cfg(test)] -mod tests { - use crate::test_connection::TestConnection; - use crate::{conn, Client}; - - fn is_send_sync(_: T) {} - - #[test] - fn construct_test_client() { - let test_conn = TestConnection::::new(vec![]); - let client = Client::new(conn::Standard::new(test_conn)); - is_send_sync(client); - } -} diff --git a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs index 356184f867..7c51dbbf65 100644 --- a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs +++ b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs @@ -7,7 +7,6 @@ use aws_auth::Credentials; use aws_endpoint::{set_endpoint_resolver, DefaultAwsEndpointResolver}; use aws_http::user_agent::AwsUserAgent; use aws_http::AwsErrorRetryPolicy; -use aws_hyper::test_connection::TestConnection; use aws_hyper::{Client, RetryConfig}; use aws_sig_auth::signer::OperationSigningConfig; use aws_types::region::Region; @@ -15,6 +14,7 @@ use aws_types::SigningService; use bytes::Bytes; use http::header::{AUTHORIZATION, HOST, USER_AGENT}; use http::{Response, Uri}; +use smithy_client::test_connection::TestConnection; use smithy_http::body::SdkBody; use smithy_http::operation; use smithy_http::operation::Operation; diff --git a/rust-runtime/smithy-client/src/bounds.rs b/rust-runtime/smithy-client/src/bounds.rs index 2c90869e28..82900df861 100644 --- a/rust-runtime/smithy-client/src/bounds.rs +++ b/rust-runtime/smithy-client/src/bounds.rs @@ -26,6 +26,7 @@ pub trait SmithyConnector: Error = ::Error, Future = ::Future, > + Send + + Sync + Clone + 'static { @@ -42,7 +43,11 @@ pub trait SmithyConnector: impl SmithyConnector for T where - T: Service, Response = http::Response> + Send + Clone + 'static, + T: Service, Response = http::Response> + + Send + + Sync + + Clone + + 'static, T::Error: Into + Send + Sync + 'static, T::Future: Send + 'static, { @@ -93,13 +98,13 @@ pub trait SmithyMiddleware: /// Forwarding type to `::Service` for bound inference. /// /// See module-level docs for details. - type Service: SmithyMiddlewareService + Send + Clone + 'static; + type Service: SmithyMiddlewareService + Send + Sync + Clone + 'static; } impl SmithyMiddleware for T where T: Layer>, - T::Service: SmithyMiddlewareService + Send + Clone + 'static, + T::Service: SmithyMiddlewareService + Send + Sync + Clone + 'static, { type Service = T::Service; } diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs index afd7a7212f..e3993ed91e 100644 --- a/rust-runtime/smithy-client/src/builder.rs +++ b/rust-runtime/smithy-client/src/builder.rs @@ -174,7 +174,7 @@ impl Builder { impl Builder where C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, + M: bounds::SmithyMiddleware + Send + Sync + 'static, R: retry::NewRequestPolicy, { /// Build a type-erased Smithy service [`Client`]. diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs index 489ac8437f..856a181552 100644 --- a/rust-runtime/smithy-client/src/erase.rs +++ b/rust-runtime/smithy-client/src/erase.rs @@ -20,7 +20,7 @@ pub type DynClient = Client Client where C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, + M: bounds::SmithyMiddleware + Send + Sync + 'static, R: retry::NewRequestPolicy, { /// Erase the middleware type from the client type signature. @@ -57,7 +57,7 @@ where impl Client where C: bounds::SmithyConnector, - M: bounds::SmithyMiddleware + Send + 'static, + M: bounds::SmithyMiddleware + Send + Sync + 'static, R: retry::NewRequestPolicy, { /// Erase the connector type from the client type signature. @@ -191,7 +191,7 @@ impl fmt::Debug for DynMiddleware { impl DynMiddleware { /// Construct a new dynamically-dispatched Smithy middleware. - pub fn new + Send + 'static>(middleware: M) -> Self { + pub fn new + Send + Sync + 'static>(middleware: M) -> Self { Self(BoxCloneLayer::new(middleware)) } } diff --git a/rust-runtime/smithy-client/src/erase/boxclone.rs b/rust-runtime/smithy-client/src/erase/boxclone.rs index d29a652801..cec7bfb371 100644 --- a/rust-runtime/smithy-client/src/erase/boxclone.rs +++ b/rust-runtime/smithy-client/src/erase/boxclone.rs @@ -9,15 +9,15 @@ use tower::layer::{layer_fn, Layer}; use tower::Service; pub(super) struct BoxCloneLayer { - boxed: Box>>, + boxed: Box> + Send + Sync>, } impl BoxCloneLayer { /// Create a new [`BoxLayer`]. pub fn new(inner_layer: L) -> Self where - L: Layer + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Future: Send + 'static, { let layer = layer_fn(move |inner: In| { @@ -51,13 +51,14 @@ trait CloneService: Service { ) -> Box< dyn CloneService + Send + + Sync + 'static, >; } impl CloneService for T where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, { fn clone_box( &self, @@ -69,7 +70,8 @@ where Future = Self::Future, > + 'static - + Send, + + Send + + Sync, > { Box::new(self.clone()) } @@ -78,7 +80,10 @@ where pub type BoxFuture = Pin> + Send>>; pub struct BoxCloneService { inner: Box< - dyn CloneService> + Send + 'static, + dyn CloneService> + + Send + + Sync + + 'static, >, } @@ -91,7 +96,7 @@ impl BoxCloneService { #[allow(missing_docs)] pub fn new(inner: S) -> Self where - S: Service + Send + 'static + Clone, + S: Service + Send + Sync + 'static + Clone, S::Future: Send + 'static, { let inner = Box::new(Boxed { inner }); diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index 9974454e66..204027a335 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -47,12 +47,30 @@ impl Builder<(), M, R> { } } -#[cfg(feature = "rustls")] -impl crate::Client>, M> { +#[cfg(any(feature = "rustls", feature = "native_tls"))] +impl crate::Client +where + M: Default, + M: crate::bounds::SmithyMiddleware + Send + Sync + 'static, +{ /// Create a Smithy client that uses HTTPS and the [standard retry - /// policy](crate::retry::Standard). - pub fn new(middleware: M) -> Self { - Builder::new().https().middleware(middleware).build() + /// policy](crate::retry::Standard) over the default middleware implementation. + /// + /// For convenience, this constructor type-erases the concrete TLS connector backend used using + /// dynamic dispatch. This comes at a slight runtime performance cost. See + /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use + /// [`Builder::rustls`] or [`Builder::native_tls`] instead. + pub fn https() -> Self { + #[cfg(feature = "rustls")] + let with_https = |b: Builder<_>| b.rustls(); + // If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled + #[cfg(not(feature = "rustls"))] + let with_https = |b: Builder<_>| b.native_tls(); + + with_https(Builder::new()) + .middleware(M::default()) + .build() + .erase_connector() } } diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index 681d57435b..1be447c3eb 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -91,11 +91,32 @@ pub struct Client< retry_policy: RetryPolicy, } +// Quick-create for people who just want "the default". +impl Client +where + M: Default, +{ + /// Create a Smithy client that the given connector, a middleware default, and the [standard + /// retry policy](crate::retry::Standard). + pub fn new(connector: C) -> Self { + Builder::new() + .connector(connector) + .middleware(M::default()) + .build() + } +} + impl Client { /// Set the standard retry policy's configuration. pub fn set_retry_config(&mut self, config: retry::Config) { self.retry_policy.with_config(config); } + + /// Adjust a standard retry client with the given policy configuration. + pub fn with_retry_config(mut self, config: retry::Config) -> Self { + self.set_retry_config(config); + self + } } impl Client diff --git a/rust-runtime/smithy-client/src/static_tests.rs b/rust-runtime/smithy-client/src/static_tests.rs index 35bdda801c..7b27345cba 100644 --- a/rust-runtime/smithy-client/src/static_tests.rs +++ b/rust-runtime/smithy-client/src/static_tests.rs @@ -83,3 +83,31 @@ fn sanity_erase_connector() { .erase_connector() .check(); } + +// Statically check that a fully type-erased client is actually a valid Client. +#[allow(dead_code)] +fn sanity_erase_full() { + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .connector_fn(|_| async { unreachable!() }) + .build() + .simplify() + .check(); +} + +fn is_send_sync(_: T) {} +fn noarg_is_send_sync() {} + +// Statically check that a fully type-erased client is still Send + Sync. +#[allow(dead_code)] +fn erased_is_send_sync() { + noarg_is_send_sync::(); + noarg_is_send_sync::>(); + is_send_sync( + Builder::new() + .middleware(tower::layer::util::Identity::new()) + .connector_fn(|_| async { unreachable!() }) + .build() + .simplify(), + ); +} From ec22f48295a13f38f992fb57452ba26b9e716195 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 15:40:37 -0700 Subject: [PATCH 16/36] Make it clear that bounds:: should never be implemented --- rust-runtime/smithy-client/src/bounds.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/rust-runtime/smithy-client/src/bounds.rs b/rust-runtime/smithy-client/src/bounds.rs index 82900df861..71026e090d 100644 --- a/rust-runtime/smithy-client/src/bounds.rs +++ b/rust-runtime/smithy-client/src/bounds.rs @@ -17,8 +17,8 @@ pub type Parsed = smithy_http_tower::parse_response::ParseResponseS /// A low-level Smithy connector that maps from [`http::Request`] to [`http::Response`]. /// -/// This trait has a blanket implementation for all compatible types, and should never need to -/// be implemented. +/// This trait has a blanket implementation for all compatible types, and should never be +/// implemented. pub trait SmithyConnector: Service< http::Request, @@ -57,8 +57,8 @@ where /// A Smithy middleware service that adjusts [`smithy_http::operation::Request`]s. /// -/// This trait has a blanket implementation for all compatible types, and should never need to -/// be implemented. +/// This trait has a blanket implementation for all compatible types, and should never be +/// implemented. pub trait SmithyMiddlewareService: Service< smithy_http::operation::Request, @@ -87,8 +87,8 @@ where /// A Smithy middleware layer (i.e., factory). /// -/// This trait has a blanket implementation for all compatible types, and should never need to -/// be implemented. +/// This trait has a blanket implementation for all compatible types, and should never be +/// implemented. pub trait SmithyMiddleware: Layer< smithy_http_tower::dispatch::DispatchService, @@ -111,8 +111,8 @@ where /// A Smithy retry policy. /// -/// This trait has a blanket implementation for all compatible types, and should never need to -/// be implemented. +/// This trait has a blanket implementation for all compatible types, and should never be +/// implemented. pub trait SmithyRetryPolicy: tower::retry::Policy, SdkSuccess, SdkError> + Clone { From e5b8ba63ae3a75fc636d004492edf630bf05601f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 15:40:46 -0700 Subject: [PATCH 17/36] Finish adjusting aws fluent generator --- aws/rust-runtime/aws-hyper/src/lib.rs | 9 ++- .../smithy/rustsdk/FluentClientGenerator.kt | 69 ++++++++++++------- aws/sdk/examples/dynamo-movies/src/main.rs | 3 +- .../secretsmanager-helloworld/src/main.rs | 4 +- 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index fdde50e6f6..ca6aac904a 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -15,8 +15,6 @@ use std::fmt::Debug; use tower::layer::util::Stack; use tower::ServiceBuilder; -pub type StandardClient = smithy_client::Client; - type AwsMiddlewareStack = Stack< MapRequestLayer, Stack, MapRequestLayer>, @@ -63,6 +61,13 @@ impl tower::Layer for AwsMiddleware { #[doc(inline)] pub type Client = smithy_client::Client; +#[doc(inline)] +pub use smithy_client::erase::DynConnector; +pub type StandardClient = Client; + +#[doc(inline)] +pub use smithy_client::bounds::SmithyConnector; + #[doc(inline)] pub type Builder = smithy_client::Builder; diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/FluentClientGenerator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/FluentClientGenerator.kt index c6d41b2b49..b88a3b60b7 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/FluentClientGenerator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/FluentClientGenerator.kt @@ -20,6 +20,7 @@ import software.amazon.smithy.rust.codegen.rustlang.documentShape import software.amazon.smithy.rust.codegen.rustlang.render import software.amazon.smithy.rust.codegen.rustlang.rust import software.amazon.smithy.rust.codegen.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.rustlang.stripOuter import software.amazon.smithy.rust.codegen.rustlang.writable @@ -80,48 +81,64 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { writer.rustTemplate( """ ##[derive(std::fmt::Debug)] - pub(crate) struct Handle { - client: #{aws_hyper}::Client<#{aws_hyper}::conn::Standard>, + pub(crate) struct Handle { + client: #{aws_hyper}::Client, conf: crate::Config } ##[derive(Clone, std::fmt::Debug)] - pub struct Client { - handle: std::sync::Arc + pub struct Client { + handle: std::sync::Arc> } """, "aws_hyper" to hyperDep.asType() ) + writer.rustBlock("impl Client") { + rustTemplate( + """ + pub fn from_conf_conn(conf: crate::Config, conn: C) -> Self { + let client = #{aws_hyper}::Client::new(conn); + Self { handle: std::sync::Arc::new(Handle { client, conf })} + } + + pub fn conf(&self) -> &crate::Config { + &self.handle.conf + } + + """, + "aws_hyper" to hyperDep.asType() + ) + } writer.rustBlock("impl Client") { rustTemplate( """ ##[cfg(any(feature = "rustls", feature = "native-tls"))] pub fn from_env() -> Self { - Self::from_conf_conn(crate::Config::builder().build(), #{aws_hyper}::conn::Standard::https()) + Self::from_conf(crate::Config::builder().build()) } ##[cfg(any(feature = "rustls", feature = "native-tls"))] pub fn from_conf(conf: crate::Config) -> Self { - Self::from_conf_conn(conf, #{aws_hyper}::conn::Standard::https()) - } - - pub fn from_conf_conn(conf: crate::Config, conn: #{aws_hyper}::conn::Standard) -> Self { - let client = #{aws_hyper}::Client::new(conn); + let client = #{aws_hyper}::Client::https(); Self { handle: std::sync::Arc::new(Handle { client, conf })} } - pub fn conf(&self) -> &crate::Config { - &self.handle.conf - } - """, "aws_hyper" to hyperDep.asType() ) + } + writer.rustBlockTemplate( + """ + impl Client + where C: #{aws_hyper}::SmithyConnector, + """, + "aws_hyper" to hyperDep.asType() + ) { operations.forEach { operation -> val name = symbolProvider.toSymbol(operation).name rust( """ - pub fn ${name.toSnakeCase()}(&self) -> fluent_builders::$name { + pub fn ${name.toSnakeCase()}(&self) -> fluent_builders::$name { fluent_builders::$name::new(self.handle.clone()) }""" ) @@ -133,24 +150,27 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { val input = operation.inputShape(model) val members: List = input.allMembers.values.toList() - rust( + rustTemplate( """ ##[derive(std::fmt::Debug)] - pub struct $name { - handle: std::sync::Arc, - inner: #T + pub struct $name { + handle: std::sync::Arc>, + inner: #{ty} }""", - input.builderSymbol(symbolProvider) + "ty" to input.builderSymbol(symbolProvider), + "aws_hyper" to hyperDep.asType() ) - rustBlock("impl $name") { + rustBlock("impl $name") { rustTemplate( """ - pub(crate) fn new(handle: std::sync::Arc) -> Self { + pub(crate) fn new(handle: std::sync::Arc>) -> Self { Self { handle, inner: Default::default() } } - pub async fn send(self) -> Result<#{ok}, #{sdk_err}<#{operation_err}>> { + pub async fn send(self) -> Result<#{ok}, #{sdk_err}<#{operation_err}>> + where C: #{aws_hyper}::SmithyConnector, + { let input = self.inner.build().map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; let op = input.make_operation(&self.handle.conf) .map_err(|err|#{sdk_err}::ConstructionFailure(err.into()))?; @@ -159,7 +179,8 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { """, "ok" to symbolProvider.toSymbol(operation.outputShape(model)), "operation_err" to operation.errorSymbol(symbolProvider), - "sdk_err" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkError") + "sdk_err" to CargoDependency.SmithyHttp(runtimeConfig).asType().copy(name = "result::SdkError"), + "aws_hyper" to hyperDep.asType() ) members.forEach { member -> val memberName = symbolProvider.toMemberName(member) diff --git a/aws/sdk/examples/dynamo-movies/src/main.rs b/aws/sdk/examples/dynamo-movies/src/main.rs index bff652adb6..2a62df5d1b 100644 --- a/aws/sdk/examples/dynamo-movies/src/main.rs +++ b/aws/sdk/examples/dynamo-movies/src/main.rs @@ -35,8 +35,7 @@ async fn main() { let conf = dynamodb::Config::builder() .region(Region::new("us-east-1")) .build(); - let conn = aws_hyper::conn::Standard::https(); - let client = dynamodb::Client::from_conf_conn(conf, conn); + let client = dynamodb::Client::from_conf(conf); let raw_client = aws_hyper::Client::https(); let table_exists = client diff --git a/aws/sdk/examples/secretsmanager-helloworld/src/main.rs b/aws/sdk/examples/secretsmanager-helloworld/src/main.rs index 0b2a2d70fb..eedd1f34a3 100644 --- a/aws/sdk/examples/secretsmanager-helloworld/src/main.rs +++ b/aws/sdk/examples/secretsmanager-helloworld/src/main.rs @@ -2,7 +2,6 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0. */ -use aws_hyper::conn::Standard; use secretsmanager::Client; use secretsmanager::Region; use secretsmanager::SdkError; @@ -23,8 +22,7 @@ async fn main() { // creds loaded from environment variables, or they can be hard coded. // Other credential providers not currently supported .build(); - let conn = Standard::https(); - let client = Client::from_conf_conn(config, conn); + let client = Client::from_conf(config); // attempt to create a secret, // need to find a better way to handle failure such as ResourceExistsException From c8c34ba341b29ec78b4f8c870873960f7d4743bb Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 15:44:02 -0700 Subject: [PATCH 18/36] Make clippy happy --- rust-runtime/smithy-client/src/erase/boxclone.rs | 2 ++ rust-runtime/smithy-client/src/hyper_impls.rs | 2 ++ rust-runtime/smithy-client/src/test_connection.rs | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/rust-runtime/smithy-client/src/erase/boxclone.rs b/rust-runtime/smithy-client/src/erase/boxclone.rs index cec7bfb371..5213217ca1 100644 --- a/rust-runtime/smithy-client/src/erase/boxclone.rs +++ b/rust-runtime/smithy-client/src/erase/boxclone.rs @@ -144,6 +144,8 @@ where { type Response = S::Response; type Error = S::Error; + + #[allow(clippy::type_complexity)] type Future = Pin> + Send>>; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index 204027a335..b48b5ea76b 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -14,6 +14,8 @@ where { type Response = http::Response; type Error = hyper::Error; + + #[allow(clippy::type_complexity)] type Future = std::pin::Pin< Box> + Send + 'static>, >; diff --git a/rust-runtime/smithy-client/src/test_connection.rs b/rust-runtime/smithy-client/src/test_connection.rs index 8d37ee6a08..7623d64b2d 100644 --- a/rust-runtime/smithy-client/src/test_connection.rs +++ b/rust-runtime/smithy-client/src/test_connection.rs @@ -128,7 +128,7 @@ where .lock() .unwrap() .push(ValidateRequest { expected, actual }); - std::future::ready(Ok(resp.map(|body| SdkBody::from(body)))) + std::future::ready(Ok(resp.map(SdkBody::from))) } else { std::future::ready(Err("No more data".into())) } From 499a96ce1670deaa20d192c8152109704affe287 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 15:44:59 -0700 Subject: [PATCH 19/36] Also re-expose test_connection in aws_hyper --- aws/rust-runtime/aws-hyper/src/lib.rs | 3 +++ aws/rust-runtime/aws-hyper/tests/e2e_test.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index ca6aac904a..18c963e8ce 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -3,6 +3,9 @@ * SPDX-License-Identifier: Apache-2.0. */ +#[doc(inline)] +pub use smithy_client::test_connection; + pub use smithy_client::retry::Config as RetryConfig; use aws_endpoint::AwsEndpointStage; diff --git a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs index 7c51dbbf65..356184f867 100644 --- a/aws/rust-runtime/aws-hyper/tests/e2e_test.rs +++ b/aws/rust-runtime/aws-hyper/tests/e2e_test.rs @@ -7,6 +7,7 @@ use aws_auth::Credentials; use aws_endpoint::{set_endpoint_resolver, DefaultAwsEndpointResolver}; use aws_http::user_agent::AwsUserAgent; use aws_http::AwsErrorRetryPolicy; +use aws_hyper::test_connection::TestConnection; use aws_hyper::{Client, RetryConfig}; use aws_sig_auth::signer::OperationSigningConfig; use aws_types::region::Region; @@ -14,7 +15,6 @@ use aws_types::SigningService; use bytes::Bytes; use http::header::{AUTHORIZATION, HOST, USER_AGENT}; use http::{Response, Uri}; -use smithy_client::test_connection::TestConnection; use smithy_http::body::SdkBody; use smithy_http::operation; use smithy_http::operation::Operation; From 7751a242196051d3dee40d42bba56804d47d59d9 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 16:37:12 -0700 Subject: [PATCH 20/36] Make ktlint happy --- .../smithy/generators/FluentClientDecorator.kt | 14 ++++---------- .../smithy/generators/HttpProtocolGenerator.kt | 6 +++--- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index 1fe63dcf83..b4c7aef111 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -26,13 +26,7 @@ import software.amazon.smithy.rust.codegen.rustlang.stripOuter import software.amazon.smithy.rust.codegen.rustlang.writable import software.amazon.smithy.rust.codegen.smithy.RustCrate import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator -import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization -import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection -import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig -import software.amazon.smithy.rust.codegen.smithy.generators.builderSymbol import software.amazon.smithy.rust.codegen.smithy.generators.error.errorSymbol -import software.amazon.smithy.rust.codegen.smithy.generators.setterName -import software.amazon.smithy.rust.codegen.smithy.RuntimeType import software.amazon.smithy.rust.codegen.smithy.rustType import software.amazon.smithy.rust.codegen.util.inputShape import software.amazon.smithy.rust.codegen.util.outputShape @@ -46,7 +40,7 @@ class FluentClientDecorator : RustCodegenDecorator { override fun extras(protocolConfig: ProtocolConfig, rustCrate: RustCrate) { if (!applies(protocolConfig)) { - return; + return } val module = RustMetadata(additionalAttributes = listOf(Attribute.Cfg.feature("client")), public = true) @@ -64,7 +58,7 @@ class FluentClientDecorator : RustCodegenDecorator { baseCustomizations: List ): List { if (!applies(protocolConfig)) { - return baseCustomizations; + return baseCustomizations } return baseCustomizations + object : LibRsCustomization() { @@ -133,7 +127,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { M: #{client}::bounds::SmithyMiddleware, R: #{client}::retry::NewRequestPolicy, """, - "client" to clientDep.asType(), + "client" to clientDep.asType(), ) { operations.forEach { operation -> val name = symbolProvider.toSymbol(operation).name @@ -169,7 +163,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { M: #{client}::bounds::SmithyMiddleware, R: #{client}::retry::NewRequestPolicy, """, - "client" to CargoDependency.SmithyClient(runtimeConfig).asType(), + "client" to CargoDependency.SmithyClient(runtimeConfig).asType(), ) { rustTemplate( """ diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt index 5f11039e9e..43ff040c52 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt @@ -148,7 +148,7 @@ abstract class HttpProtocolGenerator( val output = buildOperationTypeOutput(writer, shape) val retry = buildOperationTypeRetry(writer, features) - return with(writer) { "${format(operationT)}<$output, $retry>" }; + return with(writer) { "${format(operationT)}<$output, $retry>" } } private fun buildOperationTypeOutput( @@ -156,7 +156,7 @@ abstract class HttpProtocolGenerator( shape: OperationShape, ): String { val outputSymbol = symbolProvider.toSymbol(shape) - return with(writer) { "${format(outputSymbol)}" }; + return with(writer) { "${format(outputSymbol)}" } } private fun buildOperationTypeRetry( @@ -165,7 +165,7 @@ abstract class HttpProtocolGenerator( ): String { val retryType = features.mapNotNull { it.retryType() }.firstOrNull()?.let { writer.format(it) } ?: "()" - return with(writer) { "$retryType" }; + return with(writer) { "$retryType" } } private fun buildOperation( From 04254e1f870a090a2262b48f5988ecf5108f0d7f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 16:52:36 -0700 Subject: [PATCH 21/36] No Builder::native_tls with default features Since the function "doesn't exist", we can't link to it. Arguably, the docs should only be tested with all features enabled, but for now just don't try to link to `native_tls`. --- rust-runtime/smithy-client/src/hyper_impls.rs | 4 ++-- rust-runtime/smithy-client/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index b48b5ea76b..10f4f1f7c3 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -61,7 +61,7 @@ where /// For convenience, this constructor type-erases the concrete TLS connector backend used using /// dynamic dispatch. This comes at a slight runtime performance cost. See /// [`DynConnector`](crate::erase::DynConnector) for details. To avoid that overhead, use - /// [`Builder::rustls`] or [`Builder::native_tls`] instead. + /// [`Builder::rustls`] or `Builder::native_tls` instead. pub fn https() -> Self { #[cfg(feature = "rustls")] let with_https = |b: Builder<_>| b.rustls(); @@ -91,7 +91,7 @@ impl Builder<(), M, R> { /// Connect to the service over HTTPS using Rustls. /// /// This is exactly equivalent to [`Builder::rustls`]. If you instead wish to use `native_tls`, - /// use [`Builder::native_tls`]. + /// use `Builder::native_tls`. pub fn https( self, ) -> Builder>, M, R> diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index 1be447c3eb..f405ee919f 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -79,7 +79,7 @@ type BoxError = Box; /// With the `hyper` feature enabled, you can construct a `Client` directly from a /// [`hyper::Client`] using [`Builder::hyper`]. You can also enable the `rustls` or `native-tls` /// features to construct a Client against a standard HTTPS endpoint using [`Builder::rustls`] and -/// [`Builder::native_tls`] respectively. +/// `Builder::native_tls` respectively. #[derive(Debug)] pub struct Client< Connector = erase::DynConnector, From 20c45ddcc6cbc98e75ec95c6e4e8ee2602928464 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 4 Jun 2021 16:53:48 -0700 Subject: [PATCH 22/36] Work around rustdoc bug https://github.com/rust-lang/rust/issues/72081 --- rust-runtime/smithy-client/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust-runtime/smithy-client/src/lib.rs b/rust-runtime/smithy-client/src/lib.rs index f405ee919f..4a1fdfde4f 100644 --- a/rust-runtime/smithy-client/src/lib.rs +++ b/rust-runtime/smithy-client/src/lib.rs @@ -14,6 +14,8 @@ pub mod bounds; pub mod erase; pub mod retry; +// https://github.com/rust-lang/rust/issues/72081 +#[allow(rustdoc::private_doc_tests)] mod builder; pub use builder::Builder; From 186056e6b4ad16adf3b7e4882ba4450d0f58100f Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 09:30:00 -0700 Subject: [PATCH 23/36] Better names for type-erase methods --- rust-runtime/smithy-client/src/builder.rs | 2 +- rust-runtime/smithy-client/src/erase.rs | 14 +++++++------- rust-runtime/smithy-client/src/hyper_impls.rs | 2 +- rust-runtime/smithy-client/src/static_tests.rs | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs index e3993ed91e..5deaa1292a 100644 --- a/rust-runtime/smithy-client/src/builder.rs +++ b/rust-runtime/smithy-client/src/builder.rs @@ -198,6 +198,6 @@ where /// # client.client.check(); /// # } pub fn build_dyn(self) -> erase::DynClient { - self.build().simplify() + self.build().into_dyn() } } diff --git a/rust-runtime/smithy-client/src/erase.rs b/rust-runtime/smithy-client/src/erase.rs index 856a181552..e3176cc564 100644 --- a/rust-runtime/smithy-client/src/erase.rs +++ b/rust-runtime/smithy-client/src/erase.rs @@ -42,10 +42,10 @@ where /// .https() /// .middleware(tower::layer::util::Identity::new()) /// .build(); - /// let client = MyClient { client: client.erase_middleware() }; + /// let client = MyClient { client: client.into_dyn_middleware() }; /// # client.client.check(); /// # } - pub fn erase_middleware(self) -> Client, R> { + pub fn into_dyn_middleware(self) -> Client, R> { Client { connector: self.connector, middleware: DynMiddleware::new(self.middleware), @@ -80,10 +80,10 @@ where /// .https() /// .middleware(tower::layer::util::Identity::new()) /// .build(); - /// let client = MyClient { client: client.erase_connector() }; + /// let client = MyClient { client: client.into_dyn_connector() }; /// # client.client.check(); /// # } - pub fn erase_connector(self) -> Client { + pub fn into_dyn_connector(self) -> Client { Client { connector: DynConnector::new(self.connector), middleware: self.middleware, @@ -113,11 +113,11 @@ where /// .https() /// .middleware(tower::layer::util::Identity::new()) /// .build(); - /// let client = MyClient { client: client.simplify() }; + /// let client = MyClient { client: client.into_dyn() }; /// # client.client.check(); /// # } - pub fn simplify(self) -> DynClient { - self.erase_connector().erase_middleware() + pub fn into_dyn(self) -> DynClient { + self.into_dyn_connector().into_dyn_middleware() } } diff --git a/rust-runtime/smithy-client/src/hyper_impls.rs b/rust-runtime/smithy-client/src/hyper_impls.rs index 10f4f1f7c3..041a59bf21 100644 --- a/rust-runtime/smithy-client/src/hyper_impls.rs +++ b/rust-runtime/smithy-client/src/hyper_impls.rs @@ -72,7 +72,7 @@ where with_https(Builder::new()) .middleware(M::default()) .build() - .erase_connector() + .into_dyn_connector() } } diff --git a/rust-runtime/smithy-client/src/static_tests.rs b/rust-runtime/smithy-client/src/static_tests.rs index 7b27345cba..ebad8c062c 100644 --- a/rust-runtime/smithy-client/src/static_tests.rs +++ b/rust-runtime/smithy-client/src/static_tests.rs @@ -69,7 +69,7 @@ fn sanity_erase_middleware() { .middleware(tower::layer::util::Identity::new()) .connector_fn(|_| async { unreachable!() }) .build() - .erase_middleware() + .into_dyn_middleware() .check(); } @@ -80,7 +80,7 @@ fn sanity_erase_connector() { .middleware(tower::layer::util::Identity::new()) .connector_fn(|_| async { unreachable!() }) .build() - .erase_connector() + .into_dyn_connector() .check(); } @@ -91,7 +91,7 @@ fn sanity_erase_full() { .middleware(tower::layer::util::Identity::new()) .connector_fn(|_| async { unreachable!() }) .build() - .simplify() + .into_dyn() .check(); } @@ -108,6 +108,6 @@ fn erased_is_send_sync() { .middleware(tower::layer::util::Identity::new()) .connector_fn(|_| async { unreachable!() }) .build() - .simplify(), + .into_dyn(), ); } From cc24f668481192428cb8f8c90e90a7ffbeb2ca7c Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 10:36:12 -0700 Subject: [PATCH 24/36] Add middleware_fn --- rust-runtime/smithy-client/src/builder.rs | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs index 5deaa1292a..51bea4223b 100644 --- a/rust-runtime/smithy-client/src/builder.rs +++ b/rust-runtime/smithy-client/src/builder.rs @@ -79,6 +79,8 @@ impl Builder<(), M, R> { where F: Fn(http::Request) -> FF + Send, FF: std::future::Future, BoxError>>, + // NOTE: The extra bound here is to help the type checker give better errors earlier. + tower::util::ServiceFn: bounds::SmithyConnector, { self.connector(tower::service_fn(map)) } @@ -108,6 +110,32 @@ impl Builder { middleware, } } + + /// Use a function-like middleware that directly maps each request. + /// + /// ```rust + /// use smithy_client::Builder; + /// use smithy_http::body::SdkBody; + /// let client = Builder::new() + /// .https() + /// .middleware_fn(|req: smithy_http::operation::Request| { + /// req + /// }) + /// .build(); + /// # client.check(); + /// ``` + pub fn middleware_fn(self, map: F) -> Builder, R> + where + F: Fn(smithy_http::operation::Request) -> smithy_http::operation::Request + + Clone + + Send + + Sync + + 'static, + // NOTE: The extra bound here is to help the type checker give better errors earlier. + tower::util::MapRequestLayer: bounds::SmithyMiddleware, + { + self.middleware(tower::util::MapRequestLayer::new(map)) + } } impl Builder { From 6dcf508cc0e95904e276c86bfc6d157001c2debc Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 10:36:20 -0700 Subject: [PATCH 25/36] Better docs for client --- .../generators/FluentClientDecorator.kt | 119 +++++++++++++++++- 1 file changed, 117 insertions(+), 2 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index b4c7aef111..249878f4f7 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -65,7 +65,7 @@ class FluentClientDecorator : RustCodegenDecorator { override fun section(section: LibRsSection) = when (section) { is LibRsSection.Body -> writable { Attribute.Cfg.feature("client").render(this) - rust("pub use client::Client;") + rust("pub use client::{Client, Builder};") } else -> emptySection } @@ -81,21 +81,136 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { private val model = protocolConfig.model private val clientDep = CargoDependency.SmithyClient(protocolConfig.runtimeConfig).copy(optional = true) private val runtimeConfig = protocolConfig.runtimeConfig + private val moduleName = protocolConfig.moduleName fun render(writer: RustWriter) { writer.rustTemplate( """ - ##[derive(std::fmt::Debug)] + ##[derive(Debug)] pub(crate) struct Handle { client: #{client}::Client, conf: crate::Config, } + /// An ergonomic service client for $moduleName. + /// + /// This client allows ergonomic access to a $moduleName-shaped service. + /// Each method corresponds to an endpoint defined in the service's Smithy model, + /// and the request and response shapes are auto-generated from that same model. + /// + /// ## Constructing a Client + /// + /// To construct a client, you need a few different things: + /// + /// - A [`Config`](crate::Config) that specifies additional configuration + /// required by the service. + /// - A connector (`C`) that specifies how HTTP requests are translated + /// into HTTP responses. This will typically be an HTTP client (like + /// `hyper`), though you can also substitute in your own, like a mock + /// mock connector for testing. + /// - A "middleware" (`M`) that modifies requests prior to them being + /// sent to the request. Most commonly, middleware will decide what + /// endpoint the requests should be sent to, as well as perform + /// authentcation and authorization of requests (such as SigV4). + /// You can also have middleware that performs request/response + /// tracing, throttling, or other middleware-like tasks. + /// - A retry policy (`R`) that dictates the behavior for requests that + /// fail and should (potentially) be retried. The default type is + /// generally what you want, as it implements a well-vetted retry + /// policy described in TODO. + /// + /// To construct a client, you will generally want to call + /// [`Client::with_config`], which takes a [`#{client}::Client`] (a + /// Smithy client that isn't specialized to a particular service), + /// and a [`Config`](crate::Config). Both of these are constructed using + /// the [builder pattern] where you first construct a `Builder` type, + /// then configure it with the necessary parameters, and then call + /// `build` to construct the finalized output type. The + /// [`#{client}::Client`] builder is re-exported in this crate as + /// [`Builder`] for convenience. + /// + /// In _most_ circumstances, you will want to use the following pattern + /// to construct a client: + /// + /// ``` + /// use $moduleName::{Builder, Client}; + /// let raw_client = + /// Builder::new() + /// .https() + /// ## /* + /// .middleware(/* discussed below */) + /// ## */ + /// ## .middleware_fn(|r| r) + /// .build(); + /// let config = + /// $moduleName::Config::builder() + /// .build(); + /// let client = Client::with_config(raw_client, config); + /// ``` + /// + /// For the middleware, you'll want to use whatever matches the + /// routing, authentication and authorization required by the target + /// service. For example, for the standard AWS SDK which uses + /// [SigV4-signed requests], the middleware looks like this: + /// + // Ignored as otherwise we'd need to pull in all these dev-dependencies. + /// ```rust,ignore + /// use aws_endpoint::AwsEndpointStage; + /// use aws_http::user_agent::UserAgentStage; + /// use aws_sig_auth::middleware::SigV4SigningStage; + /// use aws_sig_auth::signer::SigV4Signer; + /// use smithy_http_tower::map_request::MapRequestLayer; + /// use tower::layer::util::Stack; + /// use tower::ServiceBuilder; + /// + /// type AwsMiddlewareStack = + /// Stack, + /// Stack, + /// MapRequestLayer>>, + /// + /// ##[derive(Debug, Default)] + /// pub struct AwsMiddleware; + /// impl tower::Layer for AwsMiddleware { + /// type Service = >::Service; + /// + /// fn layer(&self, inner: S) -> Self::Service { + /// let signer = MapRequestLayer::for_mapper(SigV4SigningStage::new(SigV4Signer::new())); _signer: MapRequestLaye + /// let endpoint_resolver = MapRequestLayer::for_mapper(AwsEndpointStage); _endpoint_resolver: MapRequestLayer + /// .layer(endpoint_resolver) _ServiceBuilder, _>> + /// .layer(user_agent) _ServiceBuilder, _>> + /// .layer(signer) _ServiceBuilder, _>> + /// .service(inner) + /// } + /// } + /// ``` + /// + /// ## Using a Client + /// + /// Once you have a client set up, you can access the service's endpoints + /// by calling the appropriate method on [`Client`]. Each such method + /// returns a request builder for that endpoint, with methods for setting + /// the various fields of the request. Once your request is complete, use + /// the `send` method to send the request. `send` returns a future, which + /// you then have to `.await` to get the service's response. + /// + /// [builder pattern]: https://rust-lang.github.io/api-guidelines/type-safety.html##c-builder + /// [SigV4-signed requests]: https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html ##[derive(Clone, std::fmt::Debug)] pub struct Client { + // TODO: Why Arc<>? handle: std::sync::Arc> } + ##[doc(inline)] + pub type Builder = #{client}::Builder; + impl From<#{client}::Client> for Client { fn from(client: #{client}::Client) -> Self { Self::with_config(client, crate::Config::builder().build()) From ddb6f63125ff30aa981637d295e0d2b143a123a4 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 10:47:19 -0700 Subject: [PATCH 26/36] Fix remaining erase_connector --- aws/rust-runtime/aws-hyper/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index 18c963e8ce..4c75bd285e 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -89,7 +89,7 @@ pub fn https() -> StandardClient { with_https(smithy_client::Builder::new()) .build() - .erase_connector() + .into_dyn_connector() } mod static_tests { From af3d6628ee2ee46b4c0fa0e2c923d656469c0486 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 10:59:49 -0700 Subject: [PATCH 27/36] Better name for service in docs --- .../rust/codegen/smithy/generators/FluentClientDecorator.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index 249878f4f7..d8ef95c292 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -82,6 +82,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { private val clientDep = CargoDependency.SmithyClient(protocolConfig.runtimeConfig).copy(optional = true) private val runtimeConfig = protocolConfig.runtimeConfig private val moduleName = protocolConfig.moduleName + private val humanName = serviceShape.id.name fun render(writer: RustWriter) { writer.rustTemplate( @@ -92,9 +93,9 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { conf: crate::Config, } - /// An ergonomic service client for $moduleName. + /// An ergonomic service client for `$humanName`. /// - /// This client allows ergonomic access to a $moduleName-shaped service. + /// This client allows ergonomic access to a `$humanName`-shaped service. /// Each method corresponds to an endpoint defined in the service's Smithy model, /// and the request and response shapes are auto-generated from that same model. /// From d5397f350d38fae0404fe5130998edc341d6c029 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 12:00:02 -0700 Subject: [PATCH 28/36] Correct send+sync test name --- .../smithy/generators/error/TopLevelErrorGeneratorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/TopLevelErrorGeneratorTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/TopLevelErrorGeneratorTest.kt index af63fc4555..f3aa04c1e9 100644 --- a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/TopLevelErrorGeneratorTest.kt +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/TopLevelErrorGeneratorTest.kt @@ -17,7 +17,7 @@ import kotlin.io.path.writeText internal class TopLevelErrorGeneratorTest { @ExperimentalPathApi @Test - fun `top level errors are sync + sync`() { + fun `top level errors are send + sync`() { val model = """ namespace com.example From f6319eebc929e457f906616910ebc7db1c908282 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 12:00:26 -0700 Subject: [PATCH 29/36] Use crate name with _ in Rust code --- .../codegen/smithy/generators/FluentClientDecorator.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index d8ef95c292..f4c7d9559a 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -82,6 +82,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { private val clientDep = CargoDependency.SmithyClient(protocolConfig.runtimeConfig).copy(optional = true) private val runtimeConfig = protocolConfig.runtimeConfig private val moduleName = protocolConfig.moduleName + private val moduleUseName = moduleName.replace("-", "_") private val humanName = serviceShape.id.name fun render(writer: RustWriter) { @@ -134,7 +135,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { /// to construct a client: /// /// ``` - /// use $moduleName::{Builder, Client}; + /// use $moduleUseName::{Builder, Client, Config}; /// let raw_client = /// Builder::new() /// .https() @@ -143,9 +144,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { /// ## */ /// ## .middleware_fn(|r| r) /// .build(); - /// let config = - /// $moduleName::Config::builder() - /// .build(); + /// let config = Config::builder().build(); /// let client = Client::with_config(raw_client, config); /// ``` /// From 9c587b7630c4fee489341ef2b4bcbf89f72b4fe9 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 7 Jun 2021 16:11:53 -0700 Subject: [PATCH 30/36] Fix up relative links The standard syntax doesn't work: https://github.com/rust-lang/rust/issues/86120 --- aws/rust-runtime/aws-hyper/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index 4c75bd285e..663edaf071 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -49,8 +49,8 @@ impl tower::Layer for AwsMiddleware { /// AWS Service Client /// /// Hyper-based AWS Service Client. Most customers will want to construct a client with -/// [`Client::https()`](Client::https). For testing & other more advanced use cases, a custom -/// connector may be used via [`Client::new(connector)`](Client::new). +/// [`Client::https`](smithy_client::Client::https). For testing & other more advanced use cases, a +/// custom connector may be used via [`Client::new(connector)`](smithy_client::Client::new). /// /// The internal connector must implement the following trait bound to be used to dispatch requests: /// ```rust,ignore From d1a599aadd4d1d4d816248b54f877c0e4540fa2b Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Tue, 8 Jun 2021 09:55:58 -0700 Subject: [PATCH 31/36] Fix the new integration test --- aws/sdk/integration-tests/kms/tests/integration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/sdk/integration-tests/kms/tests/integration.rs b/aws/sdk/integration-tests/kms/tests/integration.rs index 26a58d630d..434dae8af5 100644 --- a/aws/sdk/integration-tests/kms/tests/integration.rs +++ b/aws/sdk/integration-tests/kms/tests/integration.rs @@ -41,7 +41,7 @@ async fn generate_random_cn() { .region(Region::new("cn-north-1")) .credentials_provider(creds) .build(); - let client = kms::Client::from_conf_conn(conf, aws_hyper::conn::Standard::new(conn.clone())); + let client = kms::Client::from_conf_conn(conf, conn.clone()); let _ = client .generate_random() .number_of_bytes(64) From fd383323a7bafb9046a63388744605c6efd9ddce Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 9 Jun 2021 15:01:32 -0700 Subject: [PATCH 32/36] Hide temporary Operation type aliases --- .../rust/codegen/smithy/generators/HttpProtocolGenerator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt index 43ff040c52..091503ab55 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolGenerator.kt @@ -77,8 +77,8 @@ abstract class HttpProtocolGenerator( // on these aliases. val operationTypeOutput = buildOperationTypeOutput(inputWriter, operationShape) val operationTypeRetry = buildOperationTypeRetry(inputWriter, customizations) - inputWriter.rust("pub type ${inputShape.id.name}OperationOutputAlias= $operationTypeOutput;") - inputWriter.rust("pub type ${inputShape.id.name}OperationRetryAlias = $operationTypeRetry;") + inputWriter.rust("##[doc(hidden)] pub type ${inputShape.id.name}OperationOutputAlias= $operationTypeOutput;") + inputWriter.rust("##[doc(hidden)] pub type ${inputShape.id.name}OperationRetryAlias = $operationTypeRetry;") // impl OperationInputShape { ... } inputWriter.implBlock(inputShape, symbolProvider) { From fbffa81fdd6756f20960fbcb051488b100b6e6d7 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 10 Jun 2021 13:57:47 -0700 Subject: [PATCH 33/36] Don't bound middleware_fn as it also bounds C With the extra "helpful" bound, we also end up enforcing that C implements Service, but since we're in a builder, C may not have been set yet, and may be (), which in turn means that it isn't Service. So users would end up with an error if they write: Builder::new().middleware_fn(|r| r).https().build() but it would work with Builder::new().https().middleware_fn(|r| r).build() which is silly. --- rust-runtime/smithy-client/src/builder.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust-runtime/smithy-client/src/builder.rs b/rust-runtime/smithy-client/src/builder.rs index 51bea4223b..0281f0c1e3 100644 --- a/rust-runtime/smithy-client/src/builder.rs +++ b/rust-runtime/smithy-client/src/builder.rs @@ -131,8 +131,6 @@ impl Builder { + Send + Sync + 'static, - // NOTE: The extra bound here is to help the type checker give better errors earlier. - tower::util::MapRequestLayer: bounds::SmithyMiddleware, { self.middleware(tower::util::MapRequestLayer::new(map)) } From 5c28c250fc02f5232cb1596a72f6e84f94f9af88 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 11 Jun 2021 10:07:26 -0700 Subject: [PATCH 34/36] Don't recursive infinitely --- rust-runtime/smithy-client/src/erase/boxclone.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/smithy-client/src/erase/boxclone.rs b/rust-runtime/smithy-client/src/erase/boxclone.rs index 5213217ca1..90079ae727 100644 --- a/rust-runtime/smithy-client/src/erase/boxclone.rs +++ b/rust-runtime/smithy-client/src/erase/boxclone.rs @@ -112,7 +112,7 @@ where { fn clone(&self) -> Self { Self { - inner: self.clone_box(), + inner: self.inner.clone_box(), } } } From 7ad3790674cf938c5f4df2fd4f6aa06f1829a9c8 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 11 Jun 2021 10:09:40 -0700 Subject: [PATCH 35/36] Can only doc(inline) re-exports, not type alises --- .../rust/codegen/smithy/generators/FluentClientDecorator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index f4c7d9559a..a26de275bb 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -209,7 +209,7 @@ class FluentClientGenerator(protocolConfig: ProtocolConfig) { } ##[doc(inline)] - pub type Builder = #{client}::Builder; + pub use #{client}::Builder; impl From<#{client}::Client> for Client { fn from(client: #{client}::Client) -> Self { From 4e51f73f67f6ffb0062e671f5cbdcba2b1580ee4 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 11 Jun 2021 10:19:35 -0700 Subject: [PATCH 36/36] Can only doc(inline) re-exports, not type alises --- aws/rust-runtime/aws-hyper/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/rust-runtime/aws-hyper/src/lib.rs b/aws/rust-runtime/aws-hyper/src/lib.rs index 663edaf071..e25ff9fde1 100644 --- a/aws/rust-runtime/aws-hyper/src/lib.rs +++ b/aws/rust-runtime/aws-hyper/src/lib.rs @@ -61,7 +61,6 @@ impl tower::Layer for AwsMiddleware { /// S::Error: Into + Send + Sync + 'static, /// S::Future: Send + 'static, /// ``` -#[doc(inline)] pub type Client = smithy_client::Client; #[doc(inline)] @@ -71,7 +70,9 @@ pub type StandardClient = Client; #[doc(inline)] pub use smithy_client::bounds::SmithyConnector; -#[doc(inline)] +/// AWS Service Client builder. +/// +/// See [`smithy_client::Builder`] for details. pub type Builder = smithy_client::Builder; /// Construct an `https` based client