Unverified Commit 9c134c53 authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

Smithy 1.13 (#823) 

* Bump smithy version to 1.13

* Upgrade to Smithy 1.13

The Smith 1.13 upgrade adds new restJson protocol tests. These demonstrated two issues:

1. We were not properly using the http binding index to compute content types. We should not have fallen back to a default content type—instead we need to use exactly what is given. We were also using the index incorrectly: `document` in the context of the index refers to anything in the HTTP body, _not_ the Smithy document type.
2. We were sending `Content-Length: 0` even when no HTTP body was modeled.
3. We were sending an empty body for structures targetted `@httpPayload` trait in restJson. We should instead be sending on an empty JSON body: `{}`.

* fixup indentation

* Fix eventstream content types

* Update S3 Signing test

ListObjectsV2 was manually verified to function correctly with the updated headers.
parent 841f5111

aws/sdk/integration-tests/s3/tests/signing-it.rs

+1 −1
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ async fn test_signer() -> Result<(), aws_sdk_s3::Error> { 
        .build();

    let conn = TestConnection::new(vec![(

        http::Request::builder()

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20210618/us-east-1/s3/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-security-token;x-amz-user-agent, Signature=c55fb770a89c535e56b502f8c949766c7bde7cfc84f89d2b7761d13b8e82234c")

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20210618/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token;x-amz-user-agent, Signature=6233614b69271e15db079287874a654183916e509909b5719b00cd8d5f31299e")

            .uri("https://s3.us-east-1.amazonaws.com/test-bucket?list-type=2&prefix=prefix~")

            .body(SdkBody::empty())

            .unwrap(),

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

+2 −1
Original line number Diff line number Diff line
@@ -147,8 +147,9 @@ open class MakeOperationGenerator( 
        val contentType = httpBindingResolver.requestContentType(operationShape)

        httpBindingGenerator.renderUpdateHttpBuilder(writer)

        writer.inRequestBuilderBaseFn(inputShape) {

            Attribute.AllowUnusedMut.render(this)

            writer.rust("let mut builder = update_http_builder(input, #T::new())?;", RuntimeType.HttpRequestBuilder)

            val additionalHeaders = listOf("content-type" to contentType) + protocol.additionalHeaders(operationShape)

            val additionalHeaders = listOfNotNull(contentType?.let { "content-type" to it }) + protocol.additionalHeaders(operationShape)

            for (header in additionalHeaders) {

                writer.rustTemplate(

                    """

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

+23 −14
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import software.amazon.smithy.rust.codegen.smithy.generators.BuilderGenerator 
import software.amazon.smithy.rust.codegen.smithy.generators.FluentClientGenerator

import software.amazon.smithy.rust.codegen.smithy.generators.implBlock

import software.amazon.smithy.rust.codegen.smithy.generators.operationBuildError

import software.amazon.smithy.rust.codegen.smithy.protocols.HttpLocation

import software.amazon.smithy.rust.codegen.smithy.protocols.Protocol

import software.amazon.smithy.rust.codegen.util.inputShape
@@ -137,11 +138,13 @@ open class ProtocolGenerator( 
            )

            makeOperationGenerator.generateMakeOperation(this, operationShape, customizations)

            rustBlockTemplate(

                "fn assemble(mut builder: #{RequestBuilder}, body: #{SdkBody}) -> #{Request}<#{SdkBody}>",

                "fn assemble(builder: #{RequestBuilder}, body: #{SdkBody}) -> #{Request}<#{SdkBody}>",

                *codegenScope

            ) {

                if (needsContentLength(operationShape)) {

                    rustTemplate(

                        """

                        let mut builder = builder;

                        if let Some(content_length) = body.content_length() {

                            builder = #{header_util}::set_header_if_absent(

                                        builder,
@@ -149,11 +152,12 @@ open class ProtocolGenerator( 
                                        content_length

                            );

                        }

                    builder.body(body).expect("should be valid request")

                        """,

                        *codegenScope

                    )

                }

                rust("""builder.body(body).expect("should be valid request")""")

            }



            // pub fn builder() -> ... { }

            builderGenerator.renderConvenienceMethod(this)
@@ -188,6 +192,11 @@ open class ProtocolGenerator( 
        traitGenerator.generateTraitImpls(operationWriter, operationShape)

    }



    private fun needsContentLength(operationShape: OperationShape): Boolean {

        return protocol.httpBindingResolver.requestBindings(operationShape)

            .any { it.location == HttpLocation.DOCUMENT || it.location == HttpLocation.PAYLOAD }

    }



    private fun renderTypeAliases(

        inputWriter: RustWriter,

        operationShape: OperationShape,

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

+1 −4
Original line number Diff line number Diff line
@@ -442,10 +442,7 @@ class ProtocolTestGenerator( 
        private val RestXml = "aws.protocoltests.restxml#RestXml"

        private val AwsQuery = "aws.protocoltests.query#AwsQuery"

        private val Ec2Query = "aws.protocoltests.ec2#AwsEc2"

        private val ExpectFail = setOf<FailingTest>(

            // this is a buggy test, will be fixed in 1.13

            FailingTest(RestJson, "RestJsonNoInputAndOutput", Action.Request)

        )

        private val ExpectFail = setOf<FailingTest>()

        private val RunOnly: Set<String>? = null



        // These tests are not even attempted to be generated, either because they will not compile

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

+9 −13
Original line number Diff line number Diff line
@@ -85,30 +85,28 @@ interface HttpBindingResolver { 
    /**

     * Determines the request content type for given [operationShape].

     */

    fun requestContentType(operationShape: OperationShape): String

    fun requestContentType(operationShape: OperationShape): String?



    /**

     * Determines the response content type for given [operationShape].

     */

    fun responseContentType(operationShape: OperationShape): String

    fun responseContentType(operationShape: OperationShape): String?

}



/**

 * Content types a protocol uses.

 */

data class ProtocolContentTypes(

    /** Default request content type when the shape isn't, for example, a Blob */

    val requestDefault: String,

    /** Default response content type when the shape isn't, for example, a Blob */

    val responseDefault: String,

    /** Request content type override for when the shape is a Document */

    val requestDocument: String? = null,

    /** Response content type override for when the shape is a Document */

    val responseDocument: String? = null,

    /** EventStream content type */

    val eventStreamContentType: String? = null

) {

    companion object {

        /** Create an instance of [ProtocolContentTypes] where all content types are the same */

        fun consistent(type: String) = ProtocolContentTypes(type, type)

        fun consistent(type: String) = ProtocolContentTypes(type, type, type)

    }

}
@@ -139,13 +137,11 @@ class HttpTraitHttpBindingResolver( 
    ): TimestampFormatTrait.Format =

        httpIndex.determineTimestampFormat(memberShape, location, defaultTimestampFormat)



    override fun requestContentType(operationShape: OperationShape): String =

        httpIndex.determineRequestContentType(operationShape, contentTypes.requestDocument)

            .orElse(contentTypes.requestDefault)

    override fun requestContentType(operationShape: OperationShape): String? =

        httpIndex.determineRequestContentType(operationShape, contentTypes.requestDocument, contentTypes.eventStreamContentType).orNull()



    override fun responseContentType(operationShape: OperationShape): String =

        httpIndex.determineResponseContentType(operationShape, contentTypes.responseDocument)

            .orElse(contentTypes.responseDefault)

    override fun responseContentType(operationShape: OperationShape): String? =

        httpIndex.determineResponseContentType(operationShape, contentTypes.responseDocument, contentTypes.eventStreamContentType).orNull()



    // Sort the members after extracting them from the map to have a consistent order

    private fun mappedBindings(bindings: Map<String, HttpBinding>): List<HttpBindingDescriptor> =