From 404f402e59456c222d9c51b6d978ee04e1499778 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Fri, 17 Nov 2023 18:00:56 -0500 Subject: [PATCH] Feature-gate http versions in aws-smithy-runtime-api (#3236) ## Motivation and Context Without this, we will have http = 0.2 permanently in-tree. ## Description - Add feature gate for http 02x. - ~Add http 1x as an experiment.~ ## Testing CI ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 6 +++ aws/sdk/integration-tests/ec2/Cargo.toml | 2 +- .../codegen/core/rustlang/CargoDependency.kt | 2 +- .../aws-smithy-http-server/Cargo.toml | 2 +- rust-runtime/aws-smithy-http/Cargo.toml | 2 +- .../aws-smithy-runtime-api/Cargo.toml | 1 + .../aws-smithy-runtime-api/additional-ci | 5 ++ .../external-types-no-http.toml | 6 +++ .../src/client/interceptors/context.rs | 2 +- .../src/client/runtime_plugin.rs | 2 +- .../src/http/headers.rs | 2 + .../src/http/request.rs | 49 +++++++++++++------ .../src/http/response.rs | 14 ++++-- 13 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 rust-runtime/aws-smithy-runtime-api/external-types-no-http.toml diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 864e324bdf..3e0bec7c41 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -116,3 +116,9 @@ message = "Types/functions that were previously `#[doc(hidden)]` in `aws-config` references = ["smithy-rs#3226"] meta = { "breaking" = false, "tada" = false, "bug" = false } author = "ysaito1001" + +[[smithy-rs]] +message = "Conversions for HTTP request in aws-smithy-runtime-api are now feature gated behind the `http-02x` feature" +references = ["smithy-rs#3236"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" diff --git a/aws/sdk/integration-tests/ec2/Cargo.toml b/aws/sdk/integration-tests/ec2/Cargo.toml index 7db6addb07..b019cd5014 100644 --- a/aws/sdk/integration-tests/ec2/Cargo.toml +++ b/aws/sdk/integration-tests/ec2/Cargo.toml @@ -10,7 +10,7 @@ publish = false aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] } aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async" } aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["client", "test-util"] } -aws-smithy-runtime-api = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["client"] } +aws-smithy-runtime-api = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["client", "http-02x"] } aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" } aws-sdk-ec2 = { path = "../../build/aws-sdk/sdk/ec2", features = ["behavior-version-latest"] } tokio = { version = "1.23.1", features = ["full"]} diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt index 1a858be198..76f368fa8f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt @@ -305,7 +305,7 @@ data class CargoDependency( .withFeature("client") fun smithyRuntimeTestUtil(runtimeConfig: RuntimeConfig) = smithyRuntime(runtimeConfig).toDevDependency().withFeature("test-util") fun smithyRuntimeApi(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-runtime-api") - fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) = smithyRuntimeApi(runtimeConfig).withFeature("client") + fun smithyRuntimeApiClient(runtimeConfig: RuntimeConfig) = smithyRuntimeApi(runtimeConfig).withFeature("client").withFeature("http-02x") fun smithyRuntimeApiTestUtil(runtimeConfig: RuntimeConfig) = smithyRuntimeApi(runtimeConfig).toDevDependency().withFeature("test-util") fun smithyTypes(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-types") diff --git a/rust-runtime/aws-smithy-http-server/Cargo.toml b/rust-runtime/aws-smithy-http-server/Cargo.toml index b686cb615a..fbfcd89998 100644 --- a/rust-runtime/aws-smithy-http-server/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server/Cargo.toml @@ -21,7 +21,7 @@ request-id = ["dep:uuid"] async-trait = "0.1" aws-smithy-http = { path = "../aws-smithy-http", features = ["rt-tokio"] } aws-smithy-json = { path = "../aws-smithy-json" } -aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api" } +aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api", features = ["http-02x"] } aws-smithy-types = { path = "../aws-smithy-types", features = ["http-body-0-4-x", "hyper-0-14-x"] } aws-smithy-xml = { path = "../aws-smithy-xml" } bytes = "1.1" diff --git a/rust-runtime/aws-smithy-http/Cargo.toml b/rust-runtime/aws-smithy-http/Cargo.toml index a5bc6f4d64..2517342b79 100644 --- a/rust-runtime/aws-smithy-http/Cargo.toml +++ b/rust-runtime/aws-smithy-http/Cargo.toml @@ -16,7 +16,7 @@ rt-tokio = ["aws-smithy-types/rt-tokio"] [dependencies] aws-smithy-eventstream = { path = "../aws-smithy-eventstream", optional = true } -aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api", features = ["client"] } +aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api", features = ["client", "http-02x"] } aws-smithy-types = { path = "../aws-smithy-types", features = ["byte-stream-poll-next", "http-body-0-4-x"] } bytes = "1" bytes-utils = "0.1" diff --git a/rust-runtime/aws-smithy-runtime-api/Cargo.toml b/rust-runtime/aws-smithy-runtime-api/Cargo.toml index 3f6a96b518..3e7415448b 100644 --- a/rust-runtime/aws-smithy-runtime-api/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime-api/Cargo.toml @@ -14,6 +14,7 @@ default = [] client = [] http-auth = ["dep:zeroize"] test-util = ["aws-smithy-types/test-util"] +http-02x = [] [dependencies] aws-smithy-async = { path = "../aws-smithy-async" } diff --git a/rust-runtime/aws-smithy-runtime-api/additional-ci b/rust-runtime/aws-smithy-runtime-api/additional-ci index b44c6c05be..c27a031c39 100755 --- a/rust-runtime/aws-smithy-runtime-api/additional-ci +++ b/rust-runtime/aws-smithy-runtime-api/additional-ci @@ -8,5 +8,10 @@ set -e +# NOTE: (rcoh) This seems to be pulling in workspace settings that pull in this dependency, but it passes if +# no other crates enable this dependency +# echo "### Checking external types w/ HTTP feature disabled" +# RUSTDOCFLAGS="" cargo +"${RUST_NIGHTLY_VERSION}" check-external-types --config external-types-no-http.toml --no-default-features + echo "### Testing every combination of features (excluding --all-features)" cargo hack test --feature-powerset --exclude-all-features diff --git a/rust-runtime/aws-smithy-runtime-api/external-types-no-http.toml b/rust-runtime/aws-smithy-runtime-api/external-types-no-http.toml new file mode 100644 index 0000000000..a58f0caeda --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/external-types-no-http.toml @@ -0,0 +1,6 @@ +allowed_external_types = [ + "aws_smithy_async::*", + "aws_smithy_types::*", + + "bytes::bytes::Bytes", +] diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs index c7054c11c7..5d406bf846 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context.rs @@ -467,7 +467,7 @@ impl fmt::Display for RewindResult { } } -#[cfg(all(test, feature = "test-util"))] +#[cfg(all(test, feature = "test-util", feature = "http-02x"))] mod tests { use super::*; use aws_smithy_types::body::SdkBody; diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs index 757224051d..53812097f7 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs @@ -297,7 +297,7 @@ impl RuntimePlugins { } } -#[cfg(all(test, feature = "test-util"))] +#[cfg(all(test, feature = "test-util", feature = "http-02x"))] mod tests { use super::{RuntimePlugin, RuntimePlugins}; use crate::client::http::{ diff --git a/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs b/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs index 70361a1d86..0a2dfd617e 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http/headers.rs @@ -156,6 +156,7 @@ impl Headers { } } +#[cfg(feature = "http-02x")] impl TryFrom for Headers { type Error = HttpError; @@ -281,6 +282,7 @@ mod header_value { Ok(Self { _private: value }) } + #[allow(dead_code)] pub(crate) fn into_http02x(self) -> http0::HeaderValue { self._private } diff --git a/rust-runtime/aws-smithy-runtime-api/src/http/request.rs b/rust-runtime/aws-smithy-runtime-api/src/http/request.rs index e36f0bf28a..ee87e57f1d 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http/request.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http/request.rs @@ -10,7 +10,7 @@ use crate::http::HttpError; use aws_smithy_types::body::SdkBody; use http as http0; use http0::uri::PathAndQuery; -use http0::{Extensions, HeaderMap, Method}; +use http0::{Extensions, Method}; use std::borrow::Cow; /// Parts struct useful for structural decomposition that the [`Request`] type can be converted into. @@ -30,7 +30,7 @@ pub struct Request { body: B, uri: Uri, method: Method, - extensions: Extensions, + extensions_02x: Extensions, headers: Headers, } @@ -77,6 +77,13 @@ impl Uri { pub fn query(&self) -> Option<&str> { self.parsed.query() } + + fn from_http0x_uri(uri: http0::Uri) -> Self { + Self { + as_string: uri.to_string(), + parsed: uri, + } + } } fn merge_paths(endpoint_path: Option, uri: &http0::Uri) -> Cow<'_, str> { @@ -119,15 +126,14 @@ impl<'a> TryFrom<&'a str> for Uri { } } +#[cfg(feature = "http-02x")] impl From for Uri { fn from(value: http::Uri) -> Self { - Self { - as_string: value.to_string(), - parsed: value, - } + Uri::from_http0x_uri(value) } } +#[cfg(feature = "http-02x")] impl TryInto> for Request { type Error = HttpError; @@ -141,13 +147,15 @@ impl Request { /// /// Depending on the internal storage type, this operation may be free or it may have an internal /// cost. + #[cfg(feature = "http-02x")] pub fn try_into_http02x(self) -> Result, HttpError> { let mut req = http::Request::builder() .uri(self.uri.parsed) .method(self.method) .body(self.body) .expect("known valid"); - let mut headers = HeaderMap::new(); + let mut headers = http0::HeaderMap::new(); + headers.reserve(self.headers.headers.len()); headers.extend( self.headers .headers @@ -155,7 +163,7 @@ impl Request { .map(|(k, v)| (k, v.into_http02x())), ); *req.headers_mut() = headers; - *req.extensions_mut() = self.extensions; + *req.extensions_mut() = self.extensions_02x; Ok(req) } @@ -165,7 +173,7 @@ impl Request { body: f(self.body), uri: self.uri, method: self.method, - extensions: self.extensions, + extensions_02x: self.extensions_02x, headers: self.headers, } } @@ -174,9 +182,9 @@ impl Request { pub fn new(body: B) -> Self { Self { body, - uri: Uri::from(http0::Uri::from_static("/")), + uri: Uri::from_http0x_uri(http0::Uri::from_static("/")), method: Method::GET, - extensions: Default::default(), + extensions_02x: Default::default(), headers: Default::default(), } } @@ -242,13 +250,15 @@ impl Request { /// Adds an extension to the request extensions pub fn add_extension(&mut self, extension: T) { - self.extensions.insert(extension); + self.extensions_02x.insert(extension); } } impl Request { /// Attempts to clone this request /// + /// On clone, any extensions will be cleared. + /// /// If the body is cloneable, this will clone the request. Otherwise `None` will be returned pub fn try_clone(&self) -> Option { let body = self.body().try_clone()?; @@ -256,7 +266,7 @@ impl Request { body, uri: self.uri.clone(), method: self.method.clone(), - extensions: Extensions::new(), + extensions_02x: Extensions::new(), headers: self.headers.clone(), }) } @@ -279,23 +289,30 @@ impl Request { } } +#[cfg(feature = "http-02x")] impl TryFrom> for Request { type Error = HttpError; fn try_from(value: http::Request) -> Result { let (parts, body) = value.into_parts(); let headers = Headers::try_from(parts.headers)?; + // we need to do this eventually. + /*if !parts.extensions.is_empty() { + return Err(HttpError::new( + "Cannot convert non-empty extensions. Clear extensions before converting", + )); + }*/ Ok(Self { body, uri: parts.uri.into(), - method: parts.method.clone(), - extensions: parts.extensions, + method: parts.method, + extensions_02x: http::Extensions::new(), headers, }) } } -#[cfg(test)] +#[cfg(all(test, feature = "http-02x"))] mod test { use super::*; use aws_smithy_types::body::SdkBody; diff --git a/rust-runtime/aws-smithy-runtime-api/src/http/response.rs b/rust-runtime/aws-smithy-runtime-api/src/http/response.rs index a32b3ee415..84ad832758 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http/response.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http/response.rs @@ -5,10 +5,9 @@ //! Http Response Types -use crate::http::{HeaderValue, Headers, HttpError}; +use crate::http::{Headers, HttpError}; use aws_smithy_types::body::SdkBody; use http as http0; -use http0::{Extensions, HeaderMap}; use std::fmt; /// HTTP response status code @@ -49,6 +48,7 @@ impl TryFrom for StatusCode { } } +#[cfg(feature = "http-02x")] impl From for StatusCode { fn from(value: http0::StatusCode) -> Self { Self(value.as_u16()) @@ -73,7 +73,7 @@ pub struct Response { status: StatusCode, headers: Headers, body: B, - extensions: Extensions, + extensions: http0::Extensions, } impl Response { @@ -81,6 +81,7 @@ impl Response { /// /// Depending on the internal storage type, this operation may be free or it may have an internal /// cost. + #[cfg(feature = "http-02x")] pub fn try_into_http02x(self) -> Result, HttpError> { let mut res = http::Response::builder() .status( @@ -89,7 +90,7 @@ impl Response { ) .body(self.body) .expect("known valid"); - let mut headers = HeaderMap::new(); + let mut headers = http0::HeaderMap::new(); headers.extend( self.headers .headers @@ -169,10 +170,13 @@ impl Response { } } +#[cfg(feature = "http-02x")] impl TryFrom> for Response { type Error = HttpError; fn try_from(value: http0::Response) -> Result { + use crate::http::headers::HeaderValue; + use http0::HeaderMap; if let Some(e) = value .headers() .values() @@ -201,7 +205,7 @@ impl TryFrom> for Response { } } -#[cfg(test)] +#[cfg(all(test, feature = "http-02x"))] mod test { use super::*; use aws_smithy_types::body::SdkBody;