-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better distinguish model and HTTP plugins #2827
Merged
david-perez
merged 6 commits into
main
from
davidpz/better-distinguish-model-and-http-plugins
Jul 4, 2023
Merged
Better distinguish model and HTTP plugins #2827
david-perez
merged 6 commits into
main
from
davidpz/better-distinguish-model-and-http-plugins
Jul 4, 2023
Commits on Jul 3, 2023
-
Better distinguish model and HTTP plugins
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.
Configuration menu - View commit details
-
Copy full SHA for f865317 - Browse repository at this point
Copy the full SHA f865317View commit details -
Configuration menu - View commit details
-
Copy full SHA for a58ec48 - Browse repository at this point
Copy the full SHA a58ec48View commit details -
Configuration menu - View commit details
-
Copy full SHA for 8e1af65 - Browse repository at this point
Copy the full SHA 8e1af65View commit details -
Configuration menu - View commit details
-
Copy full SHA for ff2aab6 - Browse repository at this point
Copy the full SHA ff2aab6View commit details
Commits on Jul 4, 2023
-
Configuration menu - View commit details
-
Copy full SHA for 6caec26 - Browse repository at this point
Copy the full SHA 6caec26View commit details -
Configuration menu - View commit details
-
Copy full SHA for 113802d - Browse repository at this point
Copy the full SHA 113802dView commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.