Unverified Commit f13bb260 authored by Luca Palmieri's avatar Luca Palmieri Committed by GitHub
Browse files

Implement RFC23 - Evolve the new service builder API (#1954)



* Implement RFC 23, with the exception of PluginBuilder

* Update documentation.

* Elide implementation details in `Upgradable`.

* Update wording in docs to avoid personal pronouns.

* Update `Upgradable`'s documentation.

* Template the service builder name.

* Template MissingOperationsError directly.

* Code-generate an array directly.

* Update design/src/server/anatomy.md

Co-authored-by: default avatardavid-perez <d@vidp.dev>

* Use `rust` where we do not need templating.

* Remove unused variable.

* Add `expect` message to point users at our issue board in case a panic slips through.

* Typo.

* Update `expect` error message.

* Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call.

* Fix new pokemon bin example.

* Fix new pokemon bin example.

* Fix unresolved symbolProvider.

* Assign the `expect` error message to a variable.

* Omit additional generic parameters in Upgradable when it's first introduced.

Co-authored-by: default avatardavid-perez <d@vidp.dev>
parent 9f0bc36f
Loading
Loading
Loading
Loading
+3 −4
Original line number Diff line number Diff line
@@ -60,7 +60,7 @@ import software.amazon.smithy.rust.codegen.server.smithy.generators.protocol.Ser
 *   of `App` called `register_service()` that can be used to decorate the Python implementation
 *   of this operation.
 *
 * This class also renders the implementation of the `aws_smity_http_server_python::PyServer` trait,
 * This class also renders the implementation of the `aws_smithy_http_server_python::PyServer` trait,
 * that abstracts the processes / event loops / workers lifecycles.
 */
class PythonApplicationGenerator(
@@ -189,7 +189,7 @@ class PythonApplicationGenerator(
            ) {
                rustTemplate(
                    """
                    let builder = crate::service::$serviceName::builder();
                    let builder = crate::service::$serviceName::builder_without_plugins();
                    """,
                    *codegenScope,
                )
@@ -209,7 +209,7 @@ class PythonApplicationGenerator(
                }
                rustTemplate(
                    """
                    let mut service = #{tower}::util::BoxCloneService::new(builder.build());
                    let mut service = #{tower}::util::BoxCloneService::new(builder.build().expect("one or more operations do not have a registered handler; this is a bug in the Python code generator, please file a bug report under https://github.com/awslabs/smithy-rs/issues"));

                    {
                        use #{tower}::Layer;
@@ -224,7 +224,6 @@ class PythonApplicationGenerator(
                            service = #{tower}::util::BoxCloneService::new(layer.layer(service));
                        }
                    }

                    Ok(service)
                    """,
                    "Protocol" to protocol.markerStruct(),
+251 −171

File changed.

Preview size limit exceeded, changes collapsed.

+17 −91
Original line number Diff line number Diff line
@@ -11,9 +11,7 @@ import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.rustlang.asType
import software.amazon.smithy.rust.codegen.core.rustlang.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
@@ -53,12 +51,6 @@ interface ServerProtocol : Protocol {
    /** Returns the Rust router type. */
    fun routerType(): RuntimeType

    /**
     * Returns the construction of the `routerType` given a `ServiceShape`, a collection of operation values
     * (`self.operation_name`, ...), and the `Model`.
     */
    fun routerConstruction(operationValues: Iterable<Writable>): Writable

    /**
     * Returns the name of the constructor to be used on the `Router` type, to instantiate a `Router` using this
     * protocol.
@@ -75,6 +67,11 @@ interface ServerProtocol : Protocol {
        requestSpecModule: RuntimeType,
    ): Writable

    /**
     * Returns the Rust type of the `RequestSpec` for an operation.
     */
    fun serverRouterRequestSpecType(requestSpecModule: RuntimeType): RuntimeType

    /**
     * In some protocols, such as restJson1,
     * when there is no modeled body input, content type must not be set and the body must be empty.
@@ -98,17 +95,12 @@ class ServerAwsJsonProtocol(
    awsJsonVersion: AwsJsonVersion,
) : AwsJson(serverCodegenContext, awsJsonVersion), ServerProtocol {
    private val runtimeConfig = codegenContext.runtimeConfig
    private val codegenScope = arrayOf(
        "SmithyHttpServer" to ServerCargoDependency.SmithyHttpServer(runtimeConfig).asType(),
    )
    private val symbolProvider = codegenContext.symbolProvider
    private val service = codegenContext.serviceShape

    override fun structuredDataParser(operationShape: OperationShape): StructuredDataParserGenerator {
        fun builderSymbol(shape: StructureShape): Symbol =
            shape.serverBuilderSymbol(serverCodegenContext)
        fun returnSymbolToParse(shape: Shape): ReturnSymbolToParse =
            if (shape.canReachConstrainedShape(codegenContext.model, symbolProvider)) {
            if (shape.canReachConstrainedShape(codegenContext.model, serverCodegenContext.symbolProvider)) {
                ReturnSymbolToParse(serverCodegenContext.unconstrainedShapeSymbolProvider.toSymbol(shape), true)
            } else {
                ReturnSymbolToParse(codegenContext.symbolProvider.toSymbol(shape), false)
@@ -146,37 +138,6 @@ class ServerAwsJsonProtocol(

    override fun routerType() = RuntimeType("AwsJsonRouter", ServerCargoDependency.SmithyHttpServer(runtimeConfig), "${runtimeConfig.crateSrcPrefix}_http_server::proto::aws_json::router")

    override fun routerConstruction(operationValues: Iterable<Writable>): Writable = writable {
        val allOperationShapes = allOperations(codegenContext)

        // TODO(https://github.com/awslabs/smithy-rs/issues/1724#issue-1367509999): This causes a panic: "symbol
        // visitor should not be invoked in service shapes"
        // val serviceName = symbolProvider.toSymbol(service).name
        val serviceName = service.id.name
        val pairs = writable {
            for ((operation, operationValue) in allOperationShapes.zip(operationValues)) {
                val operationName = symbolProvider.toSymbol(operation).name
                rustTemplate(
                    """
                    (
                        String::from("$serviceName.$operationName"),
                        #{OperationValue:W}
                    ),
                    """,
                    "OperationValue" to operationValue,
                    *codegenScope,
                )
            }
        }
        rustTemplate(
            """
            #{Router}::from_iter([#{Pairs:W}])
            """,
            "Router" to routerType(),
            "Pairs" to pairs,
        )
    }

    /**
     * Returns the operation name as required by the awsJson1.x protocols.
     */
@@ -189,6 +150,11 @@ class ServerAwsJsonProtocol(
        rust("""String::from("$serviceName.$operationName")""")
    }

    override fun serverRouterRequestSpecType(
        requestSpecModule: RuntimeType,
    ): RuntimeType =
        RuntimeType("String", null, "std::string")

    override fun serverRouterRuntimeConstructor() = when (version) {
        AwsJsonVersion.Json10 -> "new_aws_json_10_router"
        AwsJsonVersion.Json11 -> "new_aws_json_11_router"
@@ -197,48 +163,6 @@ class ServerAwsJsonProtocol(

private fun restRouterType(runtimeConfig: RuntimeConfig) = RuntimeType("RestRouter", ServerCargoDependency.SmithyHttpServer(runtimeConfig), "${runtimeConfig.crateSrcPrefix}_http_server::proto::rest::router")

private fun restRouterConstruction(
    protocol: ServerProtocol,
    operationValues: Iterable<Writable>,
    codegenContext: CodegenContext,
): Writable = writable {
    val operations = allOperations(codegenContext)

    // TODO(https://github.com/awslabs/smithy-rs/issues/1724#issue-1367509999): This causes a panic: "symbol visitor
    //  should not be invoked in service shapes"
    //  val serviceName = symbolProvider.toSymbol(service).name
    val serviceName = codegenContext.serviceShape.id.name
    val pairs = writable {
        for ((operationShape, operationValue) in operations.zip(operationValues)) {
            val operationName = codegenContext.symbolProvider.toSymbol(operationShape).name
            val key = protocol.serverRouterRequestSpec(
                operationShape,
                operationName,
                serviceName,
                ServerCargoDependency.SmithyHttpServer(codegenContext.runtimeConfig).asType().member("routing::request_spec"),
            )
            rustTemplate(
                """
                (
                    #{Key:W},
                    #{OperationValue:W}
                ),
                """,
                "Key" to key,
                "OperationValue" to operationValue,
                "SmithyHttpServer" to ServerCargoDependency.SmithyHttpServer(codegenContext.runtimeConfig).asType(),
            )
        }
    }
    rustTemplate(
        """
        #{Router}::from_iter([#{Pairs:W}])
        """,
        "Router" to protocol.routerType(),
        "Pairs" to pairs,
    )
}

class ServerRestJsonProtocol(
    private val serverCodegenContext: ServerCodegenContext,
) : RestJson(serverCodegenContext), ServerProtocol {
@@ -278,8 +202,6 @@ class ServerRestJsonProtocol(

    override fun routerType() = restRouterType(runtimeConfig)

    override fun routerConstruction(operationValues: Iterable<Writable>): Writable = restRouterConstruction(this, operationValues, codegenContext)

    override fun serverRouterRequestSpec(
        operationShape: OperationShape,
        operationName: String,
@@ -287,6 +209,9 @@ class ServerRestJsonProtocol(
        requestSpecModule: RuntimeType,
    ): Writable = RestRequestSpecGenerator(httpBindingResolver, requestSpecModule).generate(operationShape)

    override fun serverRouterRequestSpecType(requestSpecModule: RuntimeType): RuntimeType =
        requestSpecModule.member("RequestSpec")

    override fun serverRouterRuntimeConstructor() = "new_rest_json_router"

    override fun serverContentTypeCheckNoModeledInput() = true
@@ -307,8 +232,6 @@ class ServerRestXmlProtocol(

    override fun routerType() = restRouterType(runtimeConfig)

    override fun routerConstruction(operationValues: Iterable<Writable>): Writable = restRouterConstruction(this, operationValues, codegenContext)

    override fun serverRouterRequestSpec(
        operationShape: OperationShape,
        operationName: String,
@@ -316,6 +239,9 @@ class ServerRestXmlProtocol(
        requestSpecModule: RuntimeType,
    ): Writable = RestRequestSpecGenerator(httpBindingResolver, requestSpecModule).generate(operationShape)

    override fun serverRouterRequestSpecType(requestSpecModule: RuntimeType): RuntimeType =
        requestSpecModule.member("RequestSpec")

    override fun serverRouterRuntimeConstructor() = "new_rest_xml_router"

    override fun serverContentTypeCheckNoModeledInput() = true
+2 −2
Original line number Diff line number Diff line
@@ -627,7 +627,7 @@ class ServerProtocolTestGenerator(
            """
            ##[allow(unused_mut)]
            let (sender, mut receiver) = #{Tokio}::sync::mpsc::channel(1);
            let service = crate::service::$serviceName::unchecked_builder()
            let service = crate::service::$serviceName::builder_without_plugins::<#{Hyper}::body::Body>()
                .$operationName(move |input: $inputT| {
                    let sender = sender.clone();
                    async move {
@@ -636,7 +636,7 @@ class ServerProtocolTestGenerator(
                        result
                    }
                })
                .build::<#{Hyper}::body::Body>();
                .build_unchecked();
            let http_response = #{Tower}::ServiceExt::oneshot(service, http_request)
                .await
                .expect("unable to make an HTTP request");
+187 −252

File changed.

Preview size limit exceeded, changes collapsed.

Loading