Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Route::new in upgrade #1891

Merged
merged 5 commits into from
Oct 26, 2022
Merged

Route::new in upgrade #1891

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6db311
Route::new in upgrade
82marbag Oct 21, 2022
5defc2a
Remove superfluous types
Oct 21, 2022
a17c083
Shorten imported types
Oct 24, 2022
b2b9272
Type alias
82marbag Oct 25, 2022
68ccc02
Merge branch 'main' into clean-builder-types
82marbag Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions .../software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGeneratorV2.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class ServerServiceGeneratorV2(
private val buildConstraints = operations.zip(builderOps).zip(extensionTypes).map { (first, exts) ->
val (operation, type) = first
// TODO(https://github.com/awslabs/smithy-rs/issues/1713#issue-1365169734): The `Error = Infallible` is an
// excess requirement to stay at parity with existing builder.
// excess requirement to stay at parity with existing builder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Formatter did this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because Intellij formats TODO lines if they are prefixed with a space, but can remove

writable {
rustTemplate(
"""
Expand All @@ -162,11 +162,7 @@ class ServerServiceGeneratorV2(
$exts,
B,
Pl,
>,
$type::Service: Clone + Send + 'static,
<$type::Service as #{Tower}::Service<#{Http}::Request<B>>>::Future: Send + 'static,

$type::Service: #{Tower}::Service<#{Http}::Request<B>, Error = std::convert::Infallible>
>
""",
"Marker" to protocol.markerStruct(),
*codegenScope,
Expand Down
4 changes: 2 additions & 2 deletions ...n/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocol.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class ServerAwsJsonProtocol(
"""
(
String::from("$serviceName.$operationName"),
#{SmithyHttpServer}::routing::Route::new(#{OperationValue:W})
#{OperationValue:W}
),
""",
"OperationValue" to operationValue,
Expand Down Expand Up @@ -184,7 +184,7 @@ private fun restRouterConstruction(
"""
(
#{Key:W},
#{SmithyHttpServer}::routing::Route::new(#{OperationValue:W})
#{OperationValue:W}
),
""",
"Key" to key,
Expand Down
23 changes: 12 additions & 11 deletions rust-runtime/aws-smithy-http-server/src/operation/upgrade.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::{
plugin::Plugin,
request::{FromParts, FromRequest},
response::IntoResponse,
routing::Route,
runtime_error::InternalFailureException,
};

Expand Down Expand Up @@ -222,10 +223,8 @@ where
/// Provides an interface to convert a representation of an operation to a HTTP [`Service`](tower::Service) with
/// canonical associated types.
pub trait Upgradable<Protocol, Operation, Exts, B, Plugin> {
type Service: Service<http::Request<B>, Response = http::Response<BoxBody>>;

/// Performs an upgrade from a representation of an operation to a HTTP [`Service`](tower::Service).
fn upgrade(self, plugin: &Plugin) -> Self::Service;
fn upgrade(self, plugin: &Plugin) -> Route<B>;
}

impl<P, Op, Exts, B, Pl, S, L, PollError> Upgradable<P, Op, Exts, B, Pl> for Operation<S, L>
Expand Down Expand Up @@ -255,17 +254,20 @@ where
// The signature of the output is correct
<Pl::Layer as Layer<Upgrade<P, Op, Exts, B, Pl::Service>>>::Service:
Service<http::Request<B>, Response = http::Response<BoxBody>>,
{
type Service = <Pl::Layer as Layer<Upgrade<P, Op, Exts, B, Pl::Service>>>::Service;

// For `Route::new` for the resulting service
<Pl::Layer as Layer<Upgrade<P, Op, Exts, B, Pl::Service>>>::Service: tower::Service<http::Request<B>, Error = std::convert::Infallible>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have tower::Service and Infallible imported here so we don't need tower::Service and std::convert::Infallible.

Same for line 261.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth adding a type alias to reduce the length of these.

<<Pl as Plugin<P, Op, S, L>>::Layer as Layer<Upgrade<P, Op, Exts, B, <Pl as Plugin<P, Op, S, L>>::Service>>>::Service: Clone + Send + 'static,
<<<Pl as Plugin<P, Op, S, L>>::Layer as Layer<Upgrade<P, Op, Exts, B, <Pl as Plugin<P, Op, S, L>>::Service>>>::Service as tower::Service<http::Request<B>>>::Future: Send + 'static,
{
/// Takes the [`Operation<S, L>`](Operation), applies [`Plugin`], then applies [`UpgradeLayer`] to
/// the modified `S`, then finally applies the modified `L`.
///
/// The composition is made explicit in the method constraints and return type.
fn upgrade(self, plugin: &Pl) -> Self::Service {
fn upgrade(self, plugin: &Pl) -> Route<B> {
let mapped = plugin.map(self);
let layer = Stack::new(UpgradeLayer::new(), mapped.layer);
layer.layer(mapped.inner)
Route::new(layer.layer(mapped.inner))
}
}

Expand All @@ -282,11 +284,10 @@ pub struct FailOnMissingOperation;
impl<P, Op, Exts, B, Pl> Upgradable<P, Op, Exts, B, Pl> for FailOnMissingOperation
where
InternalFailureException: IntoResponse<P>,
P: Send + 'static,
82marbag marked this conversation as resolved.
Show resolved Hide resolved
{
type Service = MissingFailure<P>;

fn upgrade(self, _plugin: &Pl) -> Self::Service {
MissingFailure { _protocol: PhantomData }
fn upgrade(self, _plugin: &Pl) -> Route<B> {
Route::new(MissingFailure { _protocol: PhantomData })
}
}

Expand Down