Unverified Commit 11cf41d9 authored by Harry Barber's avatar Harry Barber Committed by GitHub
Browse files

Simplify `Plugin` trait (#2740) 

## Motivation and Context

https://github.com/awslabs/smithy-rs/issues/2444

## Description

- Simplify `Plugin`, make it closer to `Layer`.
- Remove `Operation`.
parent ec45767a

CHANGELOG.next.toml

+197 −2
Original line number Diff line number Diff line
@@ -141,13 +141,13 @@ message = """`ShapeId` is the new structure used to represent a shape, with its 
Before you had an operation and an absolute name as its `NAME` member. You could apply a plugin only to some selected operation:



```

filter_by_operation_name(plugin, |name| name != Op::NAME);

filter_by_operation_name(plugin, |name| name != Op::ID);

```



Your new filter selects on an operation's absolute name, namespace or name.



```

filter_by_operation_id(plugin, |id| id.name() != Op::NAME.name());

filter_by_operation_id(plugin, |id| id.name() != Op::ID.name());

```



The above filter is applied to an operation's name, the one you use to specify the operation in the Smithy model.
@@ -168,3 +168,198 @@ message = "The occurrences of `Arc<dyn ResolveEndpoint>` have now been replaced 
references = ["smithy-rs#2758"]

meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }

author = "ysaito1001"



[[smithy-rs]]

message = """

The `Plugin` trait has now been simplified and the `Operation` struct has been removed.



## Simplication of the `Plugin` trait



Previously,



```rust

trait Plugin<P, Op, S, L> {

    type Service;

    type Layer;



    fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer>;

}

```



modified an `Operation`.



Now,



```rust

trait Plugin<Protocol, Operation, S> {

    type Service;



    fn apply(&self, svc: S) -> Self::Service;

}

```



maps a `tower::Service` to a `tower::Service`. This is equivalent to `tower::Layer` with two extra type parameters: `Protocol` and `Operation`.



The following middleware setup



```rust

pub struct PrintService<S> {

    inner: S,

    name: &'static str,

}



impl<R, S> Service<R> for PrintService<S>

where

    S: Service<R>,

{

    async fn call(&mut self, req: R) -> Self::Future {

        println!("Hi {}", self.name);

        self.inner.call(req)

    }

}



pub struct PrintLayer {

    name: &'static str,

}



impl<S> Layer<S> for PrintLayer {

    type Service = PrintService<S>;



    fn layer(&self, service: S) -> Self::Service {

        PrintService {

            inner: service,

            name: self.name,

        }

    }

}



pub struct PrintPlugin;



impl<P, Op, S, L> Plugin<P, Op, S, L> for PrintPlugin

where

    Op: OperationShape,

{

    type Service = S;

    type Layer = Stack<L, PrintLayer>;



    fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {

        input.layer(PrintLayer { name: Op::NAME })

    }

}

```



now becomes



```rust

pub struct PrintService<S> {

    inner: S,

    name: &'static str,

}



impl<R, S> Service<R> for PrintService<S>

where

    S: Service<R>,

{

    async fn call(&mut self, req: R) -> Self::Future {

        println!("Hi {}", self.name);

        self.inner.call(req)

    }

}



pub struct PrintPlugin;



impl<P, Op, S, L> Plugin<P, Op, S, L> for PrintPlugin

where

    Op: OperationShape,

{

    type Service = PrintService<S>;



    fn apply(&self, svc: S) -> Self::Service {

        PrintService { inner, name: Op::ID.name() }

    }

}

```



A single `Plugin` can no longer apply a `tower::Layer` on HTTP requests/responses _and_ modelled structures at the same time (see middleware positions [C](https://awslabs.github.io/smithy-rs/design/server/middleware.html#c-operation-specific-http-middleware) and [D](https://awslabs.github.io/smithy-rs/design/server/middleware.html#d-operation-specific-model-middleware). Instead one `Plugin` must be specified for each and passed to the service builder constructor separately:



```rust

let app = PokemonService::builder_with_plugins(/* HTTP plugins */, /* model plugins */)

    /* setters */

    .build()

    .unwrap();

```



The motivation behind this change is to simplify the job of middleware authors, separate concerns, accomodate common cases better, and to improve composition internally.



Because `Plugin` is now closer to `tower::Layer` we have two canonical converters:



```rust

use aws_smithy_http_server::plugin::{PluginLayer, LayerPlugin};



// Convert from `Layer` to `Plugin` which applies uniformly across all operations

let layer = /* some layer */;

let plugin = PluginLayer(layer);



// Convert from `Plugin` to `Layer` for some fixed protocol and operation

let plugin = /* some plugin */;

let layer = LayerPlugin::new::<SomeProtocol, SomeOperation>(plugin);

```



## Remove `Operation`



The `aws_smithy_http_server::operation::Operation` structure has now been removed. Previously, there existed a `{operation_name}_operation` setter on the service builder, which accepted an `Operation`. This allowed users to



```rust

let operation /* : Operation<_, _> */ = GetPokemonSpecies::from_service(/* tower::Service */);



let app = PokemonService::builder_without_plugins()

    .get_pokemon_species_operation(operation)

    /* other setters */

    .build()

    .unwrap();

```



to set an operation with a `tower::Service`, and



```rust

let operation /* : Operation<_, _> */ = GetPokemonSpecies::from_service(/* tower::Service */).layer(/* layer */);

let operation /* : Operation<_, _> */ = GetPokemonSpecies::from_handler(/* closure */).layer(/* layer */);



let app = PokemonService::builder_without_plugins()

    .get_pokemon_species_operation(operation)

    /* other setters */

    .build()

    .unwrap();

```



to add a `tower::Layer` (acting on HTTP requests/responses post-routing) to a single operation.



We have seen little adoption of this API and for this reason we have opted instead to introduce a new setter, accepting a `tower::Service`, on the service builder:



```rust

let app = PokemonService::builder_without_plugins()

    .get_pokemon_species_service(/* tower::Service */)

    /* other setters */

    .build()

    .unwrap();

```



Applying a `tower::Layer` to a _single_ operation is now done through the `Plugin` API:



```rust

use aws_smithy_http_server::plugin::{PluginLayer, filter_by_operation_name, IdentityPlugin};



let plugin = PluginLayer(/* layer */);

let scoped_plugin = filter_by_operation_name(plugin, |id| id == GetPokemonSpecies::ID);



let app = PokemonService::builder_with_plugins(scoped_plugin, IdentityPlugin)

    .get_pokemon_species(/* handler */)

    /* other setters */

    .build()

    .unwrap();

```



"""

references = ["smithy-rs#2740"]

meta = { "breaking" = true, "tada" = false, "bug" = false }

author = "hlbarber"

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

+4 −32
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ import software.amazon.smithy.rust.codegen.core.smithy.module 
import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait

import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticOutputTrait

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

import software.amazon.smithy.rust.codegen.core.util.toPascalCase

import software.amazon.smithy.rust.codegen.core.util.toSnakeCase

import software.amazon.smithy.rust.codegen.server.smithy.generators.DocHandlerGenerator

import software.amazon.smithy.rust.codegen.server.smithy.generators.handlerImports
@@ -73,49 +72,22 @@ class ServerModuleDocProvider(private val codegenContext: ServerCodegenContext) 
        val operations = index.getContainedOperations(codegenContext.serviceShape).toSortedSet(compareBy { it.id })



        val firstOperation = operations.first() ?: return@writable

        val firstOperationSymbol = codegenContext.symbolProvider.toSymbol(firstOperation)

        val firstOperationName = firstOperationSymbol.name.toPascalCase()

        val crateName = codegenContext.settings.moduleName.toSnakeCase()



        rustTemplate(

            """

            /// A collection of types representing each operation defined in the service closure.

            ///

            /// ## Constructing an [`Operation`](#{SmithyHttpServer}::operation::OperationShapeExt)

            ///

            /// To apply middleware to specific operations the [`Operation`](#{SmithyHttpServer}::operation::Operation)

            /// API must be used.

            ///

            /// Using the [`OperationShapeExt`](#{SmithyHttpServer}::operation::OperationShapeExt) trait

            /// implemented on each ZST we can construct an [`Operation`](#{SmithyHttpServer}::operation::Operation)

            /// with appropriate constraints given by Smithy.

            ///

            /// #### Example

            ///

            /// ```no_run

            /// use $crateName::operation_shape::$firstOperationName;

            /// use #{SmithyHttpServer}::operation::OperationShapeExt;

            ///

            #{HandlerImports:W}

            ///

            #{Handler:W}

            ///

            /// let operation = $firstOperationName::from_handler(handler)

            ///     .layer(todo!("Provide a layer implementation"));

            /// ```

            ///

            /// ## Use as Marker Structs

            ///

            /// The [plugin system](#{SmithyHttpServer}::plugin) also makes use of these

            /// The [plugin system](#{SmithyHttpServer}::plugin) makes use of these

            /// [zero-sized types](https://doc.rust-lang.org/nomicon/exotic-sizes.html##zero-sized-types-zsts) (ZSTs) to

            /// parameterize [`Plugin`](#{SmithyHttpServer}::plugin::Plugin) implementations. The traits, such as

            /// [`OperationShape`](#{SmithyHttpServer}::operation::OperationShape) can be used to provide

            /// parameterize [`Plugin`](#{SmithyHttpServer}::plugin::Plugin) implementations. Their traits, such as

            /// [`OperationShape`](#{SmithyHttpServer}::operation::OperationShape), can be used to provide

            /// operation specific information to the [`Layer`](#{Tower}::Layer) being applied.

            """.trimIndent(),

            "SmithyHttpServer" to

                ServerCargoDependency.smithyHttpServer(codegenContext.runtimeConfig).toType(),

            "Tower" to ServerCargoDependency.Tower.toType(),

            "Handler" to DocHandlerGenerator(codegenContext, firstOperation, "handler", commentToken = "///")::render,

            "Handler" to DocHandlerGenerator(codegenContext, firstOperation, "handler", commentToken = "///").docSignature(),

            "HandlerImports" to handlerImports(crateName, operations, commentToken = "///"),

        )

    }

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

+20 −11
Original line number Diff line number Diff line
@@ -6,10 +6,8 @@ 
package software.amazon.smithy.rust.codegen.server.smithy.generators



import software.amazon.smithy.model.shapes.OperationShape

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

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

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.util.inputShape
@@ -55,14 +53,25 @@ class DocHandlerGenerator( 
        }

    }



    fun render(writer: RustWriter) {

        // This assumes that the `error` (if applicable) `input`, and `output` modules have been imported by the

        // caller and hence are in scope.

        writer.rustTemplate(

    /**

     * Similarly to `docSignature`, returns the function signature of an operation handler implementation, with the

     * difference that we don't ellide the error for use in `tower::service_fn`.

     */

    fun docFixedSignature(): Writable {

        val errorT = if (operation.errors.isEmpty()) {

            "std::convert::Infallible"

        } else {

            "${ErrorModule.name}::${errorSymbol.name}"

        }



        return writable {

            rust(

                """

            #{Handler:W}

            """,

            "Handler" to docSignature(),

                $commentToken async fn $handlerName(input: ${InputModule.name}::${inputSymbol.name}) -> Result<${OutputModule.name}::${outputSymbol.name}, $errorT> {

                $commentToken     todo!()

                $commentToken }

                """.trimIndent(),

            )

        }

    }

}

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

+1 −1
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ class ServerOperationGenerator( 
            pub struct $operationName;



            impl #{SmithyHttpServer}::operation::OperationShape for $operationName {

                const NAME: #{SmithyHttpServer}::shape_id::ShapeId = #{SmithyHttpServer}::shape_id::ShapeId::new(${operationIdAbsolute.dq()}, ${operationId.namespace.dq()}, ${operationId.name.dq()});

                const ID: #{SmithyHttpServer}::shape_id::ShapeId = #{SmithyHttpServer}::shape_id::ShapeId::new(${operationIdAbsolute.dq()}, ${operationId.namespace.dq()}, ${operationId.name.dq()});



                type Input = crate::input::${operationName}Input;

                type Output = crate::output::${operationName}Output;

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

+3 −2
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ open class ServerRootGenerator( 
        val hasErrors = operations.any { it.errors.isNotEmpty() }

        val handlers: Writable = operations

            .map { operation ->

                DocHandlerGenerator(codegenContext, operation, builderFieldNames[operation]!!, "//!")::render

                DocHandlerGenerator(codegenContext, operation, builderFieldNames[operation]!!, "//!").docSignature()

            }

            .join("//!\n")
@@ -110,6 +110,7 @@ open class ServerRootGenerator( 
            //! Plugins allow you to build middleware which is aware of the operation it is being applied to.

            //!

            //! ```rust

            //! ## use #{SmithyHttpServer}::plugin::IdentityPlugin;

            //! ## use #{SmithyHttpServer}::plugin::IdentityPlugin as LoggingPlugin;

            //! ## use #{SmithyHttpServer}::plugin::IdentityPlugin as MetricsPlugin;

            //! ## use #{Hyper}::Body;
@@ -119,7 +120,7 @@ open class ServerRootGenerator( 
            //! let plugins = PluginPipeline::new()

            //!         .push(LoggingPlugin)

            //!         .push(MetricsPlugin);

            //! let builder: $builderName<Body, _> = $serviceName::builder_with_plugins(plugins);

            //! let builder: $builderName<Body, _, _> = $serviceName::builder_with_plugins(plugins, IdentityPlugin);

            //! ```

            //!

            //! Check out [`#{SmithyHttpServer}::plugin`] to learn more about plugins.