Unverified Commit d293ab34 authored by Matteo Bigoi's avatar Matteo Bigoi Committed by GitHub
Browse files

Fix errors discovered with RestJson protocol tests (#926) 



This PR fixes some errors discovered running the protocol tests for RestJson. It allows to fully build the RestJson protocol tests model, but it doesn't still generate proper tests (IE cargo test will fail) since we still haven't implemented parts for the protocol, like httpQuery trait.

The main problem discovered and fixed is that we were generating JSON deserializers for input structures without a JSON body (for traits like httpQuery that we still do not support). This was causing the in memory deduplication to get confused and the only visible effect was some clippy warning around mutable builders that shouldn't have been mutable. Without the protocol tests this would not surface since the ebs and simple model do not exercise that specific code path in the codegen.

Signed-off-by: default avatarBigo <1781140+crisidev@users.noreply.github.com>
parent 6e423810

codegen-server-test/build.gradle.kts

+1 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ data class CodegenTest(val service: String, val module: String, val extraConfig: 


val CodegenTests = listOf(

    CodegenTest("com.amazonaws.simple#SimpleService", "simple"),

    CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"),

    CodegenTest("com.amazonaws.ebs#Ebs", "ebs")

)

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

+46 −30
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import software.amazon.smithy.rust.codegen.rustlang.DependencyScope 
import software.amazon.smithy.rust.codegen.rustlang.RustMetadata

import software.amazon.smithy.rust.codegen.rustlang.RustWriter

import software.amazon.smithy.rust.codegen.rustlang.asType

import software.amazon.smithy.rust.codegen.rustlang.escape

import software.amazon.smithy.rust.codegen.rustlang.rust

import software.amazon.smithy.rust.codegen.rustlang.rustBlock

import software.amazon.smithy.rust.codegen.rustlang.rustTemplate
@@ -35,6 +36,7 @@ import software.amazon.smithy.rust.codegen.smithy.generators.Instantiator 
import software.amazon.smithy.rust.codegen.smithy.generators.protocol.ProtocolSupport

import software.amazon.smithy.rust.codegen.util.dq

import software.amazon.smithy.rust.codegen.util.getTrait

import software.amazon.smithy.rust.codegen.util.hasStreamingMember

import software.amazon.smithy.rust.codegen.util.hasTrait

import software.amazon.smithy.rust.codegen.util.inputShape

import software.amazon.smithy.rust.codegen.util.orNull
@@ -53,9 +55,13 @@ class ServerProtocolTestGenerator( 
) {

    private val logger = Logger.getLogger(javaClass.name)



    private val model = codegenContext.model

    private val inputShape = operationShape.inputShape(codegenContext.model)

    private val outputShape = operationShape.outputShape(codegenContext.model)

    private val operationSymbol = codegenContext.symbolProvider.toSymbol(operationShape)

    private val symbolProvider = codegenContext.symbolProvider

    private val operationSymbol = symbolProvider.toSymbol(operationShape)

    private val operationImplementationName = "${operationSymbol.name}${ServerHttpProtocolGenerator.OPERATION_OUTPUT_WRAPPER_SUFFIX}"

    private val operationErrorName = "crate::error::${operationSymbol.name}Error"

    private val operationIndex = OperationIndex.of(codegenContext.model)

    private val instantiator = with(codegenContext) {

        Instantiator(symbolProvider, model, runtimeConfig)
@@ -140,9 +146,16 @@ class ServerProtocolTestGenerator( 
        testCase.documentation.map {

            testModuleWriter.writeWithNoFormatting(it)

        }



        testModuleWriter.write("Test ID: ${testCase.id}")

        testModuleWriter.setNewlinePrefix("")

        testModuleWriter.writeWithNoFormatting("#[tokio::test]")

        // TODO: this allows to check-in RestJson protocol tests without

        // failures as the protocol is not fully implemented yet.

        // Remove it once the protocol is fully implemented.

        if (operationShape.id.getNamespace() == "aws.protocoltests.restjson") {

            testModuleWriter.writeWithNoFormatting("#[ignore]")

        }

        val Tokio = CargoDependency(

            "tokio",

            CratesIo("1"),
@@ -178,21 +191,25 @@ class ServerProtocolTestGenerator( 
        writeInline("let expected =")

        instantiator.render(this, inputShape, httpRequestTestCase.params)

        write(";")

        with(httpRequestTestCase) {

            host.orNull()?.also { host ->

                val withScheme = "http://$host"

        httpRequestTestCase.body.orNull()?.also { body ->

            rustTemplate(

                """

                    let mut http_request = http_request;

                    let ep = #{SmithyHttp}::endpoint::Endpoint::mutable(#{Http}::Uri::from_static(${withScheme.dq()}));

                    ep.set_endpoint(http_request.uri_mut(), parts.acquire().get());

                ##[allow(unused_mut)] let mut http_request = http::Request::builder()

                    .uri(${httpRequestTestCase.uri.dq()})

                    .body(#{SmithyHttpServer}::Body::from(#{Bytes}::from_static(b${body.dq()}))).unwrap();

                """,

                    *codegenScope,

                *codegenScope

            )

            if (!httpRequestTestCase.bodyMediaType.isEmpty()) {

                rust("""http_request.headers_mut().insert("Content-Type", http::header::HeaderValue::from_static(${httpRequestTestCase.bodyMediaType.get().dq()}));""")

            }

        }

            resolvedHost.orNull()?.also { host ->

                rust("""assert_eq!(http_request.uri().host().expect("host should be set"), ${host.dq()});""")

        if (!httpRequestTestCase.queryParams.isEmpty()) {

            val queryParams = httpRequestTestCase.queryParams.joinToString(separator = "&")

            rust("""*http_request.uri_mut() = "${httpRequestTestCase.uri}?$queryParams".parse().unwrap();""")

        }

        httpRequestTestCase.host.orNull()?.also {

            rust("""todo!("endpoint trait not supported yet");""")

        }

        checkQueryParams(this, httpRequestTestCase.queryParams)

        checkForbidQueryParams(this, httpRequestTestCase.forbidQueryParams)
@@ -242,14 +259,19 @@ class ServerProtocolTestGenerator( 
        writeInline("let output =")

        instantiator.render(this, expectedShape, testCase.params)

        write(";")

        val operationName = if (expectedShape.hasTrait<ErrorTrait>()) {

            "${operationSymbol.name}${ServerHttpProtocolGenerator.OPERATION_ERROR_WRAPPER_SUFFIX}"

        val operationImpl = if (operationShape.errors.isNotEmpty()) {

            if (expectedShape.hasTrait<ErrorTrait>()) {

                val variant = symbolProvider.toSymbol(expectedShape).name

                "$operationImplementationName::Error($operationErrorName::$variant(output))"

            } else {

                "$operationImplementationName::Output(output)"

            }

        } else {

            "${operationSymbol.name}${ServerHttpProtocolGenerator.OPERATION_OUTPUT_WRAPPER_SUFFIX}"

            "$operationImplementationName(output)"

        }

        rustTemplate(

            """

            let output = super::$operationName::Output(output);

            let output = super::$operationImpl;

            use #{Axum}::response::IntoResponse;

            let http_response = output.into_response();

            """,
@@ -263,11 +285,11 @@ class ServerProtocolTestGenerator( 
            );

            """

        )

        if (testCase.body != null) {

        if (!testCase.body.isEmpty()) {

            rustTemplate(

                """

                let body = #{Hyper}::body::to_bytes(http_response.into_body()).await.expect("unable to extract body to bytes");

                assert_eq!("${testCase.body.get().replace("\"", "\\\"")}", body);

                assert_eq!(${escape(testCase.body.get()).dq()}, body);

                """,

                *codegenScope

            )
@@ -286,11 +308,6 @@ class ServerProtocolTestGenerator( 
        val operationName = "${operationSymbol.name}${ServerHttpProtocolGenerator.OPERATION_INPUT_WRAPPER_SUFFIX}"

        rustWriter.rustTemplate(

            """

            let http_request = http::Request::builder()

                .uri(${testCase.uri.dq()})

                .header("Content-Type", ${testCase.bodyMediaType.orNull()?.dq()})

                .body(#{SmithyHttpServer}::Body::from(#{Bytes}::from_static(b${body.dq()})))

                .unwrap();

            use #{Axum}::extract::FromRequest;

            let mut http_request = #{Axum}::extract::RequestParts::new(http_request);

            let input_wrapper = super::$operationName::from_request(&mut http_request).await.expect("failed to parse request");
@@ -298,11 +315,10 @@ class ServerProtocolTestGenerator( 
            """,

            *codegenScope,

        )

        if (body == "") {

            rustWriter.write("// No body")

            rustWriter.write("assert_eq!(std::str::from_utf8(input).expect(\"`body` does not contain valid UTF-8\"), ${"".dq()});")

        if (operationShape.outputShape(model).hasStreamingMember(model)) {

            rustWriter.rust("""todo!("streaming types aren't supported yet");""")

        } else {

            rustWriter.write("assert_eq!(input, expected);")

            rustWriter.rust("assert_eq!(input, expected);")

        }

    }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt

+8 −5
Original line number Diff line number Diff line
@@ -298,12 +298,14 @@ private class ServerHttpProtocolImplGenerator( 
        val fnName = "parse_${operationShape.id.name.toSnakeCase()}_request"

        val inputShape = operationShape.inputShape(model)

        val inputSymbol = symbolProvider.toSymbol(inputShape)

        val includedMembers = httpBindingResolver.requestMembers(operationShape, HttpLocation.DOCUMENT)

        val unusedVars = if (includedMembers.isEmpty()) "##[allow(unused_variables)] " else ""

        return RuntimeType.forInlineFun(fnName, operationDeserModule) {

            Attribute.Custom("allow(clippy::unnecessary_wraps)").render(it)

            it.rustBlockTemplate(

                """

                pub async fn $fnName<B>(

                    request: &mut #{Axum}::extract::RequestParts<B>

                    ${unusedVars}request: &mut #{Axum}::extract::RequestParts<B>

                ) -> std::result::Result<

                    #{I},

                    #{SmithyRejection}
@@ -462,9 +464,9 @@ private class ServerHttpProtocolImplGenerator( 
                rustTemplate(

                    """

                    let status = output.$memberName

                        .ok_or(#{Error}::generic(${(memberName + " missing or empty").dq()}))?;

                    let http_status: u16 = #{Convert}::TryFrom::<i32>::try_from(status)

                        .map_err(|_| #{Error}::generic(${("invalid status code").dq()}))?;

                        .ok_or_else(|| #{SmithyHttpServer}::rejection::Serialize::from(${(memberName + " missing or empty").dq()}))?;

                    let http_status: u16 = std::convert::TryFrom::<i32>::try_from(status)

                        .map_err(|_| #{SmithyHttpServer}::rejection::Serialize::from(${("invalid status code").dq()}))?;

                    """.trimIndent(),

                    *codegenScope,

                )
@@ -484,7 +486,8 @@ private class ServerHttpProtocolImplGenerator( 
        val structuredDataParser = protocol.structuredDataParser(operationShape)

        Attribute.AllowUnusedMut.render(this)

        rust("let mut input = #T::default();", inputShape.builderSymbol(symbolProvider))

        structuredDataParser.serverInputParser(operationShape).also { parser ->

        val parser = structuredDataParser.serverInputParser(operationShape)

        if (parser != null) {

            rustTemplate(

                """

                let body = request.take_body().ok_or(#{SmithyHttpServer}::rejection::BodyAlreadyExtracted)?;

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

+7 −4
Original line number Diff line number Diff line
@@ -93,8 +93,8 @@ class JsonParserGenerator( 
        structureShape: StructureShape,

        includedMembers: List<MemberShape>

    ): RuntimeType {

        val unusedMut = if (includedMembers.isEmpty()) "##[allow(unused_mut)] " else ""

        return RuntimeType.forInlineFun(fnName, jsonDeserModule) {

            val unusedMut = if (includedMembers.isEmpty()) "##[allow(unused_mut)] " else ""

            it.rustBlockTemplate(

                "pub fn $fnName(value: &[u8], ${unusedMut}mut builder: #{Builder}) -> Result<#{Builder}, #{Error}>",

                "Builder" to structureShape.builderSymbol(symbolProvider),
@@ -196,10 +196,13 @@ class JsonParserGenerator( 
        )

    }



    override fun serverInputParser(operationShape: OperationShape): RuntimeType {

        val inputShape = operationShape.inputShape(model)

    override fun serverInputParser(operationShape: OperationShape): RuntimeType? {

        val includedMembers = httpBindingResolver.requestMembers(operationShape, HttpLocation.DOCUMENT)

        val fnName = symbolProvider.deserializeFunctionName(inputShape)

        if (includedMembers.isEmpty()) {

            return null

        }

        val inputShape = operationShape.inputShape(model)

        val fnName = symbolProvider.deserializeFunctionName(operationShape)

        return structureParser(fnName, inputShape, includedMembers)

    }

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

+1 −1
Original line number Diff line number Diff line
@@ -66,5 +66,5 @@ interface StructuredDataParserGenerator { 
     * }

     * ```

     */

    fun serverInputParser(operationShape: OperationShape): RuntimeType

    fun serverInputParser(operationShape: OperationShape): RuntimeType?

}