Skip to content
Unverified Commit ddba4608 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

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._
parent d7538278
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment