Remove generic body from `ParseHttpResponse` (#626)

* Remove generic body from `ParseHttpResponse`

* Update CHANGELOG

CHANGELOG.md

@@ -1,10 +1,23 @@
 vNext (Month Day Year)
 ----------------------
 
+**Breaking changes**
+
+- (#626) `ParseHttpResponse` no longer has a generic argument for the body type, but instead, always uses `SdkBody`. This may cause compilation failures for you if you are using Smithy generated types to parse JSON or XML without using a client to request data from a service. The fix should be as simple as removing `<SdkBody>` in the example below:
+
+  Before:
+  ```rust
+  let output = <Query as ParseHttpResponse<SdkBody>>::parse_loaded(&parser, &response).unwrap();
+  ```
+
+  After:
+  ```rust
+  let output = <Query as ParseHttpResponse>::parse_loaded(&parser, &response).unwrap();
+  ```
+
 **New This Week**
 - (When complete) Add profile file provider for region (#594, #xyz)
 
-
 v0.19 (August 3rd, 2021)
 ------------------------
 

aws/rust-runtime/aws-hyper/tests/e2e_test.rs

@@ -54,14 +54,11 @@ impl ProvideErrorKind for OperationError {
     }
 }
 
-impl<B> ParseHttpResponse<B> for TestOperationParser
+impl ParseHttpResponse for TestOperationParser {
-where
-    B: http_body::Body,
-{
     type Output = Result<String, OperationError>;
 
-    fn parse_unloaded(&self, _response: &mut Response<B>) -> Option<Self::Output> {
+    fn parse_unloaded(&self, response: &mut Response<SdkBody>) -> Option<Self::Output> {
-        if _response.status().is_success() {
+        if response.status().is_success() {
             Some(Ok("Hello!".to_string()))
         } else {
             Some(Err(OperationError))

aws/sdk/integration-tests/dynamodb/Cargo.toml

@@ -10,12 +10,13 @@ edition = "2018"
 [dependencies]
 aws-auth = { path = "../../build/aws-sdk/aws-auth" }
 aws-http = { path = "../../build/aws-sdk/aws-http" }
-aws-hyper = { path = "../../build/aws-sdk/aws-hyper", features = ["test-util"] }
+aws-hyper = { path = "../../build/aws-sdk/aws-hyper" }
 aws-sdk-dynamodb = { path = "../../build/aws-sdk/dynamodb" }
 bytes = "1"
 criterion = { version = "0.3.4" }
 http = "0.2.4"
 serde_json = "1"
+smithy-client = { path = "../../build/aws-sdk/smithy-client", features = ["test-util"] }
 smithy-http = { path = "../../build/aws-sdk/smithy-http" }
 smithy-types = { path = "../../build/aws-sdk/smithy-types" }
 tokio = { version = "1", features = ["full", "test-util"]}

aws/sdk/integration-tests/dynamodb/benches/deserialization_bench.rs

@@ -6,7 +6,6 @@
 use aws_sdk_dynamodb::operation::Query;
 use bytes::Bytes;
 use criterion::{criterion_group, criterion_main, Criterion};
-use smithy_http::body::SdkBody;
 use smithy_http::response::ParseHttpResponse;
 
 fn do_bench() {
@@ -23,7 +22,7 @@ fn do_bench() {
         .unwrap();
 
     let parser = Query::new();
-    let output = <Query as ParseHttpResponse<SdkBody>>::parse_loaded(&parser, &response).unwrap();
+    let output = <Query as ParseHttpResponse>::parse_loaded(&parser, &response).unwrap();
     assert_eq!(2, output.count);
 }
 

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGenerator.kt

@@ -277,14 +277,13 @@ class HttpProtocolTestGenerator(
             let parsed = parser.parse_unloaded(&mut http_response);
             let parsed = parsed.unwrap_or_else(|| {
                 let http_response = http_response.map(|body|#{bytes}::copy_from_slice(body.bytes().unwrap()));
-                <#{op} as #{parse_http_response}<#{sdk_body}>>::parse_loaded(&parser, &http_response)
+                <#{op} as #{parse_http_response}>::parse_loaded(&parser, &http_response)
             });
         """,
             "op" to operationSymbol,
             "bytes" to RuntimeType.Bytes,
             "parse_http_response" to CargoDependency.SmithyHttp(protocolConfig.runtimeConfig).asType()
                 .member("response::ParseHttpResponse"),
-            "sdk_body" to RuntimeType.sdkBody(runtimeConfig = protocolConfig.runtimeConfig)
         )
         if (expectedShape.hasTrait<ErrorTrait>()) {
             val errorSymbol = operationShape.errorSymbol(protocolConfig.symbolProvider)

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/HttpBoundProtocolGenerator.kt

@@ -245,7 +245,7 @@ class HttpBoundProtocolGenerator(
         val successCode = httpBindingResolver.httpTrait(operationShape).code
         rustTemplate(
             """
-                impl #{ParseResponse}<#{SdkBody}> for $operationName {
+                impl #{ParseResponse} for $operationName {
                     type Output = std::result::Result<#{O}, #{E}>;
                     fn parse_unloaded(&self, response: &mut http::Response<#{SdkBody}>) -> Option<Self::Output> {
                         // This is an error, defer to the non-streaming parser

codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/HttpProtocolTestGeneratorTest.kt

@@ -185,6 +185,7 @@ class HttpProtocolTestGeneratorTest {
             }
         )
         visitor.execute()
+        println("file:///$testDir/src/operation.rs")
         return testDir
     }
 
@@ -241,7 +242,6 @@ class HttpProtocolTestGeneratorTest {
 
     @Test
     fun `test invalid url parameter`() {
-
         // Hard coded implementation for this 1 test
         val path = generateService(
             """

rust-runtime/smithy-client/src/bounds.rs

@@ -119,7 +119,7 @@ pub trait SmithyRetryPolicy<O, T, E, Retry>:
     /// Forwarding type to `O` for bound inference.
     ///
     /// See module-level docs for details.
-    type O: ParseHttpResponse<SdkBody, Output = Result<T, Self::E>> + Send + Sync + Clone + 'static;
+    type O: ParseHttpResponse<Output = Result<T, Self::E>> + Send + Sync + Clone + 'static;
     /// Forwarding type to `E` for bound inference.
     ///
     /// See module-level docs for details.
@@ -134,7 +134,7 @@ pub trait SmithyRetryPolicy<O, T, E, Retry>:
 impl<R, O, T, E, Retry> SmithyRetryPolicy<O, T, E, Retry> for R
 where
     R: tower::retry::Policy<Operation<O, Retry>, SdkSuccess<T>, SdkError<E>> + Clone,
-    O: ParseHttpResponse<SdkBody, Output = Result<T, E>> + Send + Sync + Clone + 'static,
+    O: ParseHttpResponse<Output = Result<T, E>> + Send + Sync + Clone + 'static,
     E: Error + ProvideErrorKind,
     Retry: ClassifyResponse<SdkSuccess<T>, SdkError<E>>,
 {

rust-runtime/smithy-client/src/static_tests.rs

@@ -24,7 +24,7 @@ impl ProvideErrorKind for TestOperationError {
 #[derive(Clone)]
 #[non_exhaustive]
 pub struct TestOperation;
-impl ParseHttpResponse<SdkBody> for TestOperation {
+impl ParseHttpResponse for TestOperation {
     type Output = Result<(), TestOperationError>;
 
     fn parse_unloaded(&self, _: &mut http::Response<SdkBody>) -> Option<Self::Output> {

rust-runtime/smithy-http-tower/src/parse_response.rs

@@ -73,7 +73,7 @@ impl<S, O, T, E, R> tower::Service<operation::Operation<O, R>> for ParseResponse
 where
     S: Service<operation::Request, Response = http::Response<SdkBody>, Error = SendOperationError>,
     S::Future: Send + 'static,
-    O: ParseHttpResponse<SdkBody, Output = Result<T, E>> + Send + Sync + 'static,
+    O: ParseHttpResponse<Output = Result<T, E>> + Send + Sync + 'static,
     E: Error,
 {
     type Response = smithy_http::result::SdkSuccess<T>;

rust-runtime/smithy-http/src/middleware.rs

@@ -90,7 +90,7 @@ pub async fn load_response<T, E, O>(
     handler: &O,
 ) -> Result<SdkSuccess<T>, SdkError<E>>
 where
-    O: ParseHttpResponse<SdkBody, Output = Result<T, E>>,
+    O: ParseHttpResponse<Output = Result<T, E>>,
 {
     if let Some(parsed_response) = handler.parse_unloaded(&mut response) {
         trace!(response = ?response);

rust-runtime/smithy-http/src/response.rs

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0.
  */
 
+use crate::body::SdkBody;
 use bytes::Bytes;
 use http::Response;
 
@@ -18,7 +19,7 @@ use http::Response;
 ///
 /// It also enables this critical and core trait to avoid being async, and it makes code that uses
 /// the trait easier to test.
-pub trait ParseHttpResponse<B> {
+pub trait ParseHttpResponse {
     /// Output type of the HttpResponse.
     ///
     /// For request/response style operations, this is typically something like:
@@ -44,7 +45,7 @@ pub trait ParseHttpResponse<B> {
     /// the streaming body with an empty body as long as the body implements default.
     ///
     /// We should consider if this is too limiting & if this should take an owned response instead.
-    fn parse_unloaded(&self, response: &mut http::Response<B>) -> Option<Self::Output>;
+    fn parse_unloaded(&self, response: &mut http::Response<SdkBody>) -> Option<Self::Output>;
 
     /// Parse an HTTP request from a fully loaded body. This is for standard request/response style
     /// APIs like AwsJson 1.0/1.1 and the error path of most streaming APIs
@@ -69,13 +70,10 @@ pub trait ParseStrictResponse {
     fn parse(&self, response: &Response<Bytes>) -> Self::Output;
 }
 
-impl<B, T> ParseHttpResponse<B> for T
+impl<T: ParseStrictResponse> ParseHttpResponse for T {
-where
-    T: ParseStrictResponse,
-{
     type Output = T::Output;
 
-    fn parse_unloaded(&self, _response: &mut Response<B>) -> Option<Self::Output> {
+    fn parse_unloaded(&self, _response: &mut Response<SdkBody>) -> Option<Self::Output> {
         None
     }
 
@@ -86,30 +84,26 @@ where
 
 #[cfg(test)]
 mod test {
+    use crate::body::SdkBody;
     use crate::response::ParseHttpResponse;
     use bytes::Bytes;
     use http::Response;
-    use http_body::Body;
     use std::mem;
 
     #[test]
     fn supports_streaming_body() {
-        pub struct S3GetObject<B: Body> {
+        pub struct S3GetObject {
-            pub body: B,
+            pub body: SdkBody,
         }
 
         struct S3GetObjectParser;
 
-        impl<B> ParseHttpResponse<B> for S3GetObjectParser
+        impl ParseHttpResponse for S3GetObjectParser {
-        where
+            type Output = S3GetObject;
-            B: Default + Body,
-        {
-            type Output = S3GetObject<B>;
 
-            fn parse_unloaded(&self, response: &mut Response<B>) -> Option<Self::Output> {
+            fn parse_unloaded(&self, response: &mut Response<SdkBody>) -> Option<Self::Output> {
-                // For responses that pass on the body, use mem::take to leave behind an empty
+                // For responses that pass on the body, use mem::take to leave behind an empty body
-                // body
+                let body = mem::replace(response.body_mut(), SdkBody::taken());
-                let body = mem::take(response.body_mut());
                 Some(S3GetObject { body })
             }