Better distinguish model and HTTP plugins (#2827)

So far, servers have tacitly worked with the notion that plugins come in
two flavors: "HTTP plugins" and "model plugins":

- A HTTP plugin acts on the HTTP request before it is deserialized, and
  acts on the HTTP response after it is serialized.
- A model plugin acts on the modeled operation input after it is
  deserialized, and acts on the modeled operation output or the modeled
  operation error before it is serialized.

However, this notion was never reified in the type system. Thus, users
who pass in a model plugin where a HTTP plugin is expected or
viceversa in several APIs:

```rust
let pipeline = PluginPipeline::new().push(http_plugin).push(model_plugin);

let app = SimpleService::builder_with_plugins(http_plugins, IdentityPlugin)
    .post_operation(handler)
    /* ... */
    .build()
    .unwrap();
```

would get the typical Tower service compilation errors, which can get
very confusing:

```
error[E0631]: type mismatch in function arguments
  --> simple/rust-server-codegen/src/main.rs:47:6
   |
15 | fn new_svc<S, Ext>(inner: S) -> model_plugin::PostOperationService<S, Ext> {
   | -------------------------------------------------------------------------- found signature defined here
...
47 |     .post_operation(post_operation)
   |      ^^^^^^^^^^^^^^ expected due to this
   |
   = note: expected function signature `fn(Upgrade<RestJson1, (PostOperationInput, _), PostOperationService<aws_smithy_http_server::operation::IntoService<simple::operation_shape::PostOperation, _>, _>>) -> _`
              found function signature `fn(aws_smithy_http_server::operation::IntoService<simple::operation_shape::PostOperation, _>) -> _`
   = note: required for `LayerFn<fn(aws_smithy_http_server::operation::IntoService<..., ...>) -> ... {new_svc::<..., ...>}>` to implement `Layer<Upgrade<RestJson1, (PostOperationInput, _), PostOperationService<aws_smithy_http_server::operation::IntoService<simple::operation_shape::PostOperation, _>, _>>>`
   = note: the full type name has been written to '/local/home/davidpz/workplace/smithy-ws/src/SmithyRsSource/target/debug/deps/simple-6577f9f79749ceb9.long-type-4938700695428041031.txt'
```

This commit introduces the `HttpPlugin` and `ModelPlugin` marker traits,
allowing plugins to be marked as an HTTP plugin, a model plugin, or
both. It also removes the primary way of concatenating plugins,
`PluginPipeline`, in favor of `HttpPlugins` and `ModelPlugins`, which
eagerly check that whenever a plugin is `push`ed, it is of the expected
type. The generated service type in the server SDKs'
`builder_with_plugins` constructor now takes an `HttpPlugin` as its
first parameter, and a `ModelPlugin` as its second parameter.

I think this change perhaps goes counter to the generally accepted
wisdom that trait bounds in Rust should be enforced "at the latest
possible moment", that is, only when the behavior encoded in the trait
implementation is relied upon in the code (citation needed).
However, the result is that exposing the concepts of HTTP plugin and
model plugin in the type system makes for code that is easier to reason
about, and produces more helpful compiler error messages.

Documentation about the plugin system has been expanded, particularly on
how to implement model plugins.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

Author: david-perez

CHANGELOG.next.toml

@@ -210,6 +210,7 @@ message = """The middleware system has been reworked as we push for a unified, s
 
 - A `ServiceShape` trait has been added.
 - The `Plugin` trait has been simplified.
+- The `HttpMarker` and `ModelMarker` marker traits have been added to better distinguish when plugins run and what they have access to.
 - The `Operation` structure has been removed.
 - A `Scoped` `Plugin` has been added.
 
@@ -371,6 +372,8 @@ where
         PrintService { inner, name: Op::ID.name() }
     }
 }
+
+impl HttpMarker for PrintPlugin { }
 ```
 
 Alternatively, using the new `ServiceShape`, implemented on `Ser`:
@@ -397,6 +400,11 @@ let app = PokemonService::builder_with_plugins(/* HTTP plugins */, /* model plug
     .unwrap();
 ```
 
+To better distinguish when a plugin runs and what it has access to, `Plugin`s now have to additionally implement the `HttpMarker` marker trait, the `ModelMarker` marker trait, or both:
+
+- A HTTP plugin acts on the HTTP request before it is deserialized, and acts on the HTTP response after it is serialized.
+- A model plugin acts on the modeled operation input after it is deserialized, and acts on the modeled operation output or the modeled operation error before it is serialized.
+
 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:
@@ -413,6 +421,34 @@ let plugin = /* some plugin */;
 let layer = LayerPlugin::new::<SomeProtocol, SomeOperation>(plugin);
 ```
 
+## Removal of `PluginPipeline`
+
+Since plugins now come in two flavors (those marked with `HttpMarker` and those marked with `ModelMarker`) that shouldn't be mixed in a collection of plugins, the primary way of concatenating plugins, `PluginPipeline` has been removed in favor of the `HttpPlugins` and `ModelPlugins` types, which eagerly check that whenever a plugin is pushed, it is of the expected type.
+
+This worked before, but you wouldn't be able to do apply this collection of plugins anywhere; if you tried to, the compilation error messages would not be very helpful:
+
+```rust
+use aws_smithy_http_server::plugin::PluginPipeline;
+
+let pipeline = PluginPipeline::new().push(http_plugin).push(model_plugin);
+```
+
+Now collections of plugins must contain plugins of the same flavor:
+
+```rust
+use aws_smithy_http_server::plugin::{HttpPlugins, ModelPlugins};
+
+let http_plugins = HttpPlugins::new()
+    .push(http_plugin)
+    // .push(model_plugin) // This fails to compile with a helpful error message.
+    .push(&http_and_model_plugin);
+let model_plugins = ModelPlugins::new()
+    .push(model_plugin)
+    .push(&http_and_model_plugin);
+```
+
+In the above example, `&http_and_model_plugin` implements both `HttpMarker` and `ModelMarker`, so we can add it to both collections.
+
 ## Removal of `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
@@ -495,8 +531,8 @@ let scoped_plugin = Scoped::new::<SomeScope>(plugin);
 ```
 
 """
-references = ["smithy-rs#2740", "smithy-rs#2759", "smithy-rs#2779"]
-meta = { "breaking" = true, "tada" = false, "bug" = false }
+references = ["smithy-rs#2740", "smithy-rs#2759", "smithy-rs#2779", "smithy-rs#2827"]
+meta = { "breaking" = true, "tada" = true, "bug" = false }
 author = "hlbarber"
 
 [[smithy-rs]]

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

@@ -106,21 +106,22 @@ open class ServerRootGenerator(
             //! #### Plugins
             //!
             //! The [`$serviceName::builder_with_plugins`] method, returning [`$builderName`],
-            //! accepts a [`Plugin`](aws_smithy_http_server::plugin::Plugin).
+            //! accepts a plugin marked with [`HttpMarker`](aws_smithy_http_server::plugin::HttpMarker) and a 
+            //! plugin marked with [`ModelMarker`](aws_smithy_http_server::plugin::ModelMarker).
             //! 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;
-            //! use #{SmithyHttpServer}::plugin::PluginPipeline;
+            //! use #{SmithyHttpServer}::plugin::HttpPlugins;
             //! use $crateName::{$serviceName, $builderName};
             //!
-            //! let plugins = PluginPipeline::new()
+            //! let http_plugins = HttpPlugins::new()
             //!         .push(LoggingPlugin)
             //!         .push(MetricsPlugin);
-            //! let builder: $builderName<Body, _, _> = $serviceName::builder_with_plugins(plugins, IdentityPlugin);
+            //! let builder: $builderName<Body, _, _> = $serviceName::builder_with_plugins(http_plugins, IdentityPlugin);
             //! ```
             //!
             //! Check out [`#{SmithyHttpServer}::plugin`] to learn more about plugins.

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

@@ -9,14 +9,12 @@ import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
 import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
 import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
 import software.amazon.smithy.rust.codegen.server.smithy.ServerCargoDependency
-import software.amazon.smithy.rust.codegen.server.smithy.ServerRuntimeType
 
 class ServerRuntimeTypesReExportsGenerator(
     codegenContext: CodegenContext,
 ) {
     private val runtimeConfig = codegenContext.runtimeConfig
     private val codegenScope = arrayOf(
-        "Router" to ServerRuntimeType.router(runtimeConfig),
         "SmithyHttpServer" to ServerCargoDependency.smithyHttpServer(runtimeConfig).toType(),
     )
 
@@ -30,8 +28,11 @@ class ServerRuntimeTypesReExportsGenerator(
                 pub use #{SmithyHttpServer}::operation::OperationShape;
             }
             pub mod plugin {
+                pub use #{SmithyHttpServer}::plugin::HttpPlugins;
+                pub use #{SmithyHttpServer}::plugin::ModelPlugins;
+                pub use #{SmithyHttpServer}::plugin::HttpMarker;
+                pub use #{SmithyHttpServer}::plugin::ModelMarker;
                 pub use #{SmithyHttpServer}::plugin::Plugin;
-                pub use #{SmithyHttpServer}::plugin::PluginPipeline;
                 pub use #{SmithyHttpServer}::plugin::PluginStack;
             }
             pub mod request {

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

@@ -136,31 +136,31 @@ class ServerServiceGenerator(
                 where
                     HandlerType: #{SmithyHttpServer}::operation::Handler<crate::operation_shape::$structName, HandlerExtractors>,
 
-                    ModelPlugin: #{SmithyHttpServer}::plugin::Plugin<
+                    ModelPl: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
                         #{SmithyHttpServer}::operation::IntoService<crate::operation_shape::$structName, HandlerType>
                     >,
                     #{SmithyHttpServer}::operation::UpgradePlugin::<UpgradeExtractors>: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
-                        ModelPlugin::Output
+                        ModelPl::Output
                     >,
-                    HttpPlugin: #{SmithyHttpServer}::plugin::Plugin<
+                    HttpPl: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
                         <
                             #{SmithyHttpServer}::operation::UpgradePlugin::<UpgradeExtractors>
                             as #{SmithyHttpServer}::plugin::Plugin<
                                 $serviceName,
                                 crate::operation_shape::$structName,
-                                ModelPlugin::Output
+                                ModelPl::Output
                             >
                         >::Output
                     >,
 
-                    HttpPlugin::Output: #{Tower}::Service<#{Http}::Request<Body>, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static,
-                    <HttpPlugin::Output as #{Tower}::Service<#{Http}::Request<Body>>>::Future: Send + 'static,
+                    HttpPl::Output: #{Tower}::Service<#{Http}::Request<Body>, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static,
+                    <HttpPl::Output as #{Tower}::Service<#{Http}::Request<Body>>>::Future: Send + 'static,
 
                 {
                     use #{SmithyHttpServer}::operation::OperationShapeExt;
@@ -199,31 +199,31 @@ class ServerServiceGenerator(
                 where
                     S: #{SmithyHttpServer}::operation::OperationService<crate::operation_shape::$structName, ServiceExtractors>,
 
-                    ModelPlugin: #{SmithyHttpServer}::plugin::Plugin<
+                    ModelPl: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
                         #{SmithyHttpServer}::operation::Normalize<crate::operation_shape::$structName, S>
                     >,
                     #{SmithyHttpServer}::operation::UpgradePlugin::<UpgradeExtractors>: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
-                        ModelPlugin::Output
+                        ModelPl::Output
                     >,
-                    HttpPlugin: #{SmithyHttpServer}::plugin::Plugin<
+                    HttpPl: #{SmithyHttpServer}::plugin::Plugin<
                         $serviceName,
                         crate::operation_shape::$structName,
                         <
                             #{SmithyHttpServer}::operation::UpgradePlugin::<UpgradeExtractors>
                             as #{SmithyHttpServer}::plugin::Plugin<
                                 $serviceName,
                                 crate::operation_shape::$structName,
-                                ModelPlugin::Output
+                                ModelPl::Output
                             >
                         >::Output
                     >,
 
-                    HttpPlugin::Output: #{Tower}::Service<#{Http}::Request<Body>, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static,
-                    <HttpPlugin::Output as #{Tower}::Service<#{Http}::Request<Body>>>::Future: Send + 'static,
+                    HttpPl::Output: #{Tower}::Service<#{Http}::Request<Body>, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static,
+                    <HttpPl::Output as #{Tower}::Service<#{Http}::Request<Body>>>::Future: Send + 'static,
 
                 {
                     use #{SmithyHttpServer}::operation::OperationShapeExt;
@@ -394,16 +394,16 @@ class ServerServiceGenerator(
 
     /** Returns a `Writable` containing the builder struct definition and its implementations. */
     private fun builder(): Writable = writable {
-        val builderGenerics = listOf(builderBodyGenericTypeName, "HttpPlugin", "ModelPlugin").joinToString(", ")
+        val builderGenerics = listOf(builderBodyGenericTypeName, "HttpPl", "ModelPl").joinToString(", ")
         rustTemplate(
             """
             /// The service builder for [`$serviceName`].
             ///
             /// Constructed via [`$serviceName::builder_with_plugins`] or [`$serviceName::builder_without_plugins`].
             pub struct $builderName<$builderGenerics> {
                 ${builderFields.joinToString(", ")},
-                http_plugin: HttpPlugin,
-                model_plugin: ModelPlugin
+                http_plugin: HttpPl,
+                model_plugin: ModelPl
             }
 
             impl<$builderGenerics> $builderName<$builderGenerics> {
@@ -473,9 +473,10 @@ class ServerServiceGenerator(
                 ///
                 /// Use [`$serviceName::builder_without_plugins`] if you don't need to apply plugins.
                 ///
-                /// Check out [`PluginPipeline`](#{SmithyHttpServer}::plugin::PluginPipeline) if you need to apply
+                /// Check out [`HttpPlugins`](#{SmithyHttpServer}::plugin::HttpPlugins) and
+                /// [`ModelPlugins`](#{SmithyHttpServer}::plugin::ModelPlugins) if you need to apply
                 /// multiple plugins.
-                pub fn builder_with_plugins<Body, HttpPlugin, ModelPlugin>(http_plugin: HttpPlugin, model_plugin: ModelPlugin) -> $builderName<Body, HttpPlugin, ModelPlugin> {
+                pub fn builder_with_plugins<Body, HttpPl: #{SmithyHttpServer}::plugin::HttpMarker, ModelPl: #{SmithyHttpServer}::plugin::ModelMarker>(http_plugin: HttpPl, model_plugin: ModelPl) -> $builderName<Body, HttpPl, ModelPl> {
                     $builderName {
                         #{NotSetFields:W},
                         http_plugin,

design/src/server/anatomy.md

@@ -466,40 +466,40 @@ stateDiagram-v2
 The service builder API requires plugins to be specified upfront - they must be passed as an argument to `builder_with_plugins` and cannot be modified afterwards.
 
 You might find yourself wanting to apply _multiple_ plugins to your service.
-This can be accommodated via [`PluginPipeline`].
+This can be accommodated via [`HttpPlugins`] and [`ModelPlugins`].
 
 ```rust
 # extern crate aws_smithy_http_server;
-use aws_smithy_http_server::plugin::PluginPipeline;
+use aws_smithy_http_server::plugin::HttpPlugins;
 # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin;
 # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin;
 
-let pipeline = PluginPipeline::new().push(LoggingPlugin).push(MetricsPlugin);
+let http_plugins = HttpPlugins::new().push(LoggingPlugin).push(MetricsPlugin);
 ```
 
 The plugins' runtime logic is executed in registration order.
 In the example above, `LoggingPlugin` would run first, while `MetricsPlugin` is executed last.
 
-If you are vending a plugin, you can leverage `PluginPipeline` as an extension point: you can add custom methods to it using an extension trait.
+If you are vending a plugin, you can leverage `HttpPlugins` or `ModelPlugins` as an extension point: you can add custom methods to it using an extension trait.
 For example:
 
 ```rust
 # extern crate aws_smithy_http_server;
-use aws_smithy_http_server::plugin::{PluginPipeline, PluginStack};
+use aws_smithy_http_server::plugin::{HttpPlugins, PluginStack};
 # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin;
 # use aws_smithy_http_server::plugin::IdentityPlugin as AuthPlugin;
 
 pub trait AuthPluginExt<CurrentPlugins> {
-    fn with_auth(self) -> PluginPipeline<PluginStack<AuthPlugin, CurrentPlugins>>;
+    fn with_auth(self) -> HttpPlugins<PluginStack<AuthPlugin, CurrentPlugins>>;
 }
 
-impl<CurrentPlugins> AuthPluginExt<CurrentPlugins> for PluginPipeline<CurrentPlugins> {
-    fn with_auth(self) -> PluginPipeline<PluginStack<AuthPlugin, CurrentPlugins>> {
+impl<CurrentPlugins> AuthPluginExt<CurrentPlugins> for HttpPlugins<CurrentPlugins> {
+    fn with_auth(self) -> HttpPlugins<PluginStack<AuthPlugin, CurrentPlugins>> {
         self.push(AuthPlugin)
     }
 }
 
-let pipeline = PluginPipeline::new()
+let http_plugins = HttpPlugins::new()
     .push(LoggingPlugin)
     // Our custom method!
     .with_auth();
@@ -518,15 +518,15 @@ You can create an instance of a service builder by calling either `builder_witho
 /// The service builder for [`PokemonService`].
 ///
 /// Constructed via [`PokemonService::builder`].
-pub struct PokemonServiceBuilder<Body, HttpPlugin, ModelPlugin> {
+pub struct PokemonServiceBuilder<Body, HttpPl, ModelPl> {
     capture_pokemon_operation: Option<Route<Body>>,
     empty_operation: Option<Route<Body>>,
     get_pokemon_species: Option<Route<Body>>,
     get_server_statistics: Option<Route<Body>>,
     get_storage: Option<Route<Body>>,
     health_check_operation: Option<Route<Body>>,
-    http_plugin: HttpPlugin,
-    model_plugin: ModelPlugin
+    http_plugin: HttpPl,
+    model_plugin: ModelPl,
 }
 ```
 
@@ -537,7 +537,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g
     where
         HandlerType:Handler<GetPokemonSpecies, HandlerExtractors>,
 
-        ModelPlugin: Plugin<
+        ModelPl: Plugin<
             PokemonService,
             GetPokemonSpecies,
             IntoService<GetPokemonSpecies, HandlerType>
@@ -547,7 +547,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g
             GetPokemonSpecies,
             ModelPlugin::Output
         >,
-        HttpPlugin: Plugin<
+        HttpPl: Plugin<
             PokemonService,
             GetPokemonSpecies,
             UpgradePlugin::<UpgradeExtractors>::Output
@@ -565,7 +565,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g
     where
         S: OperationService<GetPokemonSpecies, ServiceExtractors>,
 
-        ModelPlugin: Plugin<
+        ModelPl: Plugin<
             PokemonService,
             GetPokemonSpecies,
             Normalize<GetPokemonSpecies, S>
@@ -575,7 +575,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g
             GetPokemonSpecies,
             ModelPlugin::Output
         >,
-        HttpPlugin: Plugin<
+        HttpPl: Plugin<
             PokemonService,
             GetPokemonSpecies,
             UpgradePlugin::<UpgradeExtractors>::Output

design/src/server/instrumentation.md

@@ -66,11 +66,11 @@ This is enabled via the `instrument` method provided by the `aws_smithy_http_ser
 # let handler = |req: GetPokemonSpeciesInput| async { Result::<GetPokemonSpeciesOutput, GetPokemonSpeciesError>::Ok(todo!()) };
 use aws_smithy_http_server::{
   instrumentation::InstrumentExt,
-  plugin::{IdentityPlugin, PluginPipeline}
+  plugin::{IdentityPlugin, HttpPlugins}
 };
 use pokemon_service_server_sdk::PokemonService;
 
-let http_plugins = PluginPipeline::new().instrument();
+let http_plugins = HttpPlugins::new().instrument();
 let app = PokemonService::builder_with_plugins(http_plugins, IdentityPlugin)
   .get_pokemon_species(handler)
   /* ... */