Skip to content

Commit

Permalink
Feature-gate http versions in aws-smithy-runtime-api (#3236)
Browse files Browse the repository at this point in the history
## 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
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [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._
  • Loading branch information
rcoh authored Nov 17, 2023
1 parent f17aeb4 commit 404f402
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 27 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 1 addition & 1 deletion aws/sdk/integration-tests/ec2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-http-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions rust-runtime/aws-smithy-runtime-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
5 changes: 5 additions & 0 deletions rust-runtime/aws-smithy-runtime-api/additional-ci
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
allowed_external_types = [
"aws_smithy_async::*",
"aws_smithy_types::*",

"bytes::bytes::Bytes",
]
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
2 changes: 2 additions & 0 deletions rust-runtime/aws-smithy-runtime-api/src/http/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl Headers {
}
}

#[cfg(feature = "http-02x")]
impl TryFrom<HeaderMap> for Headers {
type Error = HttpError;

Expand Down Expand Up @@ -281,6 +282,7 @@ mod header_value {
Ok(Self { _private: value })
}

#[allow(dead_code)]
pub(crate) fn into_http02x(self) -> http0::HeaderValue {
self._private
}
Expand Down
49 changes: 33 additions & 16 deletions rust-runtime/aws-smithy-runtime-api/src/http/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,7 +30,7 @@ pub struct Request<B = SdkBody> {
body: B,
uri: Uri,
method: Method,
extensions: Extensions,
extensions_02x: Extensions,
headers: Headers,
}

Expand Down Expand Up @@ -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<PathAndQuery>, uri: &http0::Uri) -> Cow<'_, str> {
Expand Down Expand Up @@ -119,15 +126,14 @@ impl<'a> TryFrom<&'a str> for Uri {
}
}

#[cfg(feature = "http-02x")]
impl From<http0::Uri> 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<B> TryInto<http0::Request<B>> for Request<B> {
type Error = HttpError;

Expand All @@ -141,21 +147,23 @@ impl<B> Request<B> {
///
/// 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<http0::Request<B>, 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
.into_iter()
.map(|(k, v)| (k, v.into_http02x())),
);
*req.headers_mut() = headers;
*req.extensions_mut() = self.extensions;
*req.extensions_mut() = self.extensions_02x;
Ok(req)
}

Expand All @@ -165,7 +173,7 @@ impl<B> Request<B> {
body: f(self.body),
uri: self.uri,
method: self.method,
extensions: self.extensions,
extensions_02x: self.extensions_02x,
headers: self.headers,
}
}
Expand All @@ -174,9 +182,9 @@ impl<B> Request<B> {
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(),
}
}
Expand Down Expand Up @@ -242,21 +250,23 @@ impl<B> Request<B> {

/// Adds an extension to the request extensions
pub fn add_extension<T: Send + Sync + Clone + 'static>(&mut self, extension: T) {
self.extensions.insert(extension);
self.extensions_02x.insert(extension);
}
}

impl Request<SdkBody> {
/// 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<Self> {
let body = self.body().try_clone()?;
Some(Self {
body,
uri: self.uri.clone(),
method: self.method.clone(),
extensions: Extensions::new(),
extensions_02x: Extensions::new(),
headers: self.headers.clone(),
})
}
Expand All @@ -279,23 +289,30 @@ impl Request<SdkBody> {
}
}

#[cfg(feature = "http-02x")]
impl<B> TryFrom<http0::Request<B>> for Request<B> {
type Error = HttpError;

fn try_from(value: http::Request<B>) -> Result<Self, Self::Error> {
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;
Expand Down
14 changes: 9 additions & 5 deletions rust-runtime/aws-smithy-runtime-api/src/http/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -49,6 +48,7 @@ impl TryFrom<u16> for StatusCode {
}
}

#[cfg(feature = "http-02x")]
impl From<http0::StatusCode> for StatusCode {
fn from(value: http0::StatusCode) -> Self {
Self(value.as_u16())
Expand All @@ -73,14 +73,15 @@ pub struct Response<B = SdkBody> {
status: StatusCode,
headers: Headers,
body: B,
extensions: Extensions,
extensions: http0::Extensions,
}

impl<B> Response<B> {
/// Converts this response into an http 0.x 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<http0::Response<B>, HttpError> {
let mut res = http::Response::builder()
.status(
Expand All @@ -89,7 +90,7 @@ impl<B> Response<B> {
)
.body(self.body)
.expect("known valid");
let mut headers = HeaderMap::new();
let mut headers = http0::HeaderMap::new();
headers.extend(
self.headers
.headers
Expand Down Expand Up @@ -169,10 +170,13 @@ impl Response<SdkBody> {
}
}

#[cfg(feature = "http-02x")]
impl<B> TryFrom<http0::Response<B>> for Response<B> {
type Error = HttpError;

fn try_from(value: http0::Response<B>) -> Result<Self, Self::Error> {
use crate::http::headers::HeaderValue;
use http0::HeaderMap;
if let Some(e) = value
.headers()
.values()
Expand Down Expand Up @@ -201,7 +205,7 @@ impl<B> TryFrom<http0::Response<B>> for Response<B> {
}
}

#[cfg(test)]
#[cfg(all(test, feature = "http-02x"))]
mod test {
use super::*;
use aws_smithy_types::body::SdkBody;
Expand Down

0 comments on commit 404f402

Please sign in to comment.