Unverified Commit 1f31eae3 authored by 82marbag's avatar 82marbag Committed by GitHub
Browse files

Correctly generate http response codes (#1405) 

* Correctly generate http response codes

http response codes will be:
* `@httpResponseCode` if set
* `@http`'s code, if `@httpResponseCode` is not set
* the default one, if neither of the above are set

@httpResponseCode honors @required and generates code that
doesn't use Option when applied.

Closes: #1229
parent a5790f7e

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

+39 −7
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import software.amazon.smithy.model.shapes.StructureShape 
import software.amazon.smithy.model.traits.EnumTrait

import software.amazon.smithy.model.traits.ErrorTrait

import software.amazon.smithy.model.traits.HttpErrorTrait

import software.amazon.smithy.model.traits.HttpTrait

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

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

import software.amazon.smithy.rust.codegen.rustlang.RustModule
@@ -50,6 +51,7 @@ import software.amazon.smithy.rust.codegen.smithy.generators.protocol.MakeOperat 
import software.amazon.smithy.rust.codegen.smithy.generators.protocol.ProtocolGenerator

import software.amazon.smithy.rust.codegen.smithy.generators.protocol.ProtocolTraitImplGenerator

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

import software.amazon.smithy.rust.codegen.smithy.isOptional

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

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

import software.amazon.smithy.rust.codegen.smithy.protocols.HttpLocation
@@ -468,7 +470,15 @@ private class ServerHttpBoundProtocolTraitImplGenerator( 
        Attribute.AllowUnusedMut.render(this)

        rustTemplate("let mut builder = #{http}::Response::builder();", *codegenScope)

        serverRenderResponseHeaders(operationShape)

        bindings.find { it.location == HttpLocation.RESPONSE_CODE }?.let { serverRenderResponseCodeBinding(it)(this) }

        bindings.find { it.location == HttpLocation.RESPONSE_CODE }

            ?.let {

                serverRenderResponseCodeBinding(it)(this)

            }

            // no binding, use http's

            ?: operationShape.getTrait<HttpTrait>()?.code?.let {

                serverRenderHttpResponseCode(it)(this)

            }

        // Fallback to the default code of `http::response::Builder`, 200.



        operationShape.outputShape(model).findStreamingMember(model)?.let {

            val memberName = symbolProvider.toMemberName(it)
@@ -551,25 +561,47 @@ private class ServerHttpBoundProtocolTraitImplGenerator( 
        }

    }



    private fun serverRenderHttpResponseCode(

        defaultCode: Int

    ): Writable {

        return writable {

            rustTemplate(

                """

                let status = $defaultCode;

                let http_status: u16 = status.try_into()

                    .map_err(|_| #{ResponseRejection}::InvalidHttpStatusCode)?;

                builder = builder.status(http_status);

                """.trimIndent(),

                *codegenScope,

            )

        }

    }



    private fun serverRenderResponseCodeBinding(

        binding: HttpBindingDescriptor

    ): Writable {

        check(binding.location == HttpLocation.RESPONSE_CODE)



        return writable {

            val memberName = symbolProvider.toMemberName(binding.member)

            // TODO(https://github.com/awslabs/smithy-rs/issues/1229): This code is problematic for two reasons:

            // 1. We're not falling back to the `http` trait if no `output.$memberName` is `None`.

            // 2. It only works when `output.$memberName` is of type `Option<i32>`.

            rust("let status = output.$memberName")

            if (symbolProvider.toSymbol(binding.member).isOptional()) {

                rustTemplate(

                    """

                    .ok_or(#{ResponseRejection}::MissingHttpStatusCode)?

                    """.trimIndent(),

                    *codegenScope,

                )

            }

            rustTemplate(

                """

                let status = output.$memberName

                    .ok_or(#{ResponseRejection}::MissingHttpStatusCode)?;

                ;

                let http_status: u16 = status.try_into()

                    .map_err(|_| #{ResponseRejection}::InvalidHttpStatusCode)?;

                builder = builder.status(http_status);

                """.trimIndent(),

                *codegenScope,

            )

            rust("builder = builder.status(http_status);")

        }

    }

codegen-test/model/misc.smithy

+71 −0
Original line number Diff line number Diff line
@@ -3,6 +3,8 @@ $version: "1.0" 
namespace aws.protocoltests.misc



use aws.protocols#restJson1

use smithy.test#httpRequestTests

use smithy.test#httpResponseTests



/// A service to test miscellaneous aspects of code generation where protocol

/// selection is not relevant. If you want to test something protocol-specific,
@@ -12,6 +14,9 @@ use aws.protocols#restJson1 
service MiscService {

    operations: [

        OperationWithInnerRequiredShape,

        ResponseCodeRequired,

        ResponseCodeHttpFallback,

        ResponseCodeDefault,

    ],

}
@@ -108,3 +113,69 @@ union AUnion { 
    string: String,

    time: Timestamp,

}



/// This operation tests that the response code defaults to 200 when no other code is set

@httpResponseTests([

    {

        id: "ResponseCodeDefault",

        protocol: "aws.protocols#restJson1",

        code: 200,

    }

])

@http(method: "GET", uri: "/responseCodeDefault")

operation ResponseCodeDefault {

    input: ResponseCodeDefaultInput,

    output: ResponseCodeDefaultOutput,

}



@input

structure ResponseCodeDefaultInput {}



@output

structure ResponseCodeDefaultOutput {}



/// This operation tests that the response code defaults to @http's code

@httpResponseTests([

    {

        id: "ResponseCodeHttpFallback",

        protocol: "aws.protocols#restJson1",

        code: 418,

    }

])

@http(method: "GET", uri: "/responseCodeHttpFallback", code: 418)

operation ResponseCodeHttpFallback {

    input: ResponseCodeHttpFallbackInput,

    output: ResponseCodeHttpFallbackOutput,

}



@input

structure ResponseCodeHttpFallbackInput {}



@output

structure ResponseCodeHttpFallbackOutput {}



/// This operation tests that @httpResponseCode is @required

/// and is used over @http's code

@httpResponseTests([

    {

        id: "ResponseCodeRequired",

        protocol: "aws.protocols#restJson1",

        code: 201,

        params: {"responseCode": 201}

    }

])

@http(method: "GET", uri: "/responseCodeRequired", code: 200)

operation ResponseCodeRequired {

    input: ResponseCodeRequiredInput,

    output: ResponseCodeRequiredOutput,

}



@input

structure ResponseCodeRequiredInput {}



@output

structure ResponseCodeRequiredOutput {

    @required

    @httpResponseCode

    responseCode: Integer,

}