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.

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

OperationShape::NAME as ShapeId #2678

Merged
merged 18 commits into from
May 11, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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.dq
import software.amazon.smithy.rust.codegen.core.util.toPascalCase
import software.amazon.smithy.rust.codegen.server.smithy.ServerCargoDependency

Expand Down Expand Up @@ -49,12 +50,13 @@ class ServerOperationGenerator(
val requestFmt = generator.requestFmt()
val responseFmt = generator.responseFmt()

val operationIdAbsolute = operationId.toString().replace("#", "##")
writer.rustTemplate(
"""
pub struct $operationName;

impl #{SmithyHttpServer}::operation::OperationShape for $operationName {
const NAME: &'static str = "${operationId.toString().replace("#", "##")}";
const NAME: #{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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,13 +478,13 @@ class ServerServiceGeneratorV2(
}

private fun missingOperationsError(): Writable = writable {
rust(
rustTemplate(
"""
/// The error encountered when calling the [`$builderName::build`] method if one or more operation handlers are not
/// specified.
##[derive(Debug)]
pub struct MissingOperationsError {
operation_names2setter_methods: std::collections::HashMap<&'static str, &'static str>,
operation_names2setter_methods: std::collections::HashMap<#{SmithyHttpServer}::shape_id::ShapeId, &'static str>,
}

impl std::fmt::Display for MissingOperationsError {
Expand All @@ -495,7 +495,7 @@ class ServerServiceGeneratorV2(
We are missing handlers for the following operations:\n",
)?;
for operation_name in self.operation_names2setter_methods.keys() {
writeln!(f, "- {}", operation_name)?;
writeln!(f, "- {}", operation_name.absolute())?;
}

writeln!(f, "\nUse the dedicated methods on `$builderName` to register the missing handlers:")?;
Expand All @@ -508,6 +508,7 @@ class ServerServiceGeneratorV2(

impl std::error::Error for MissingOperationsError {}
""",
*codegenScope,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use aws_smithy_http_server::{
operation::{Operation, OperationShape},
plugin::{Plugin, PluginPipeline, PluginStack},
shape_id::ShapeId,
};
use tower::{layer::util::Stack, Layer, Service};

Expand All @@ -17,7 +18,7 @@ use std::task::{Context, Poll};
#[derive(Clone, Debug)]
pub struct PrintService<S> {
inner: S,
name: &'static str,
id: ShapeId,
}

impl<R, S> Service<R> for PrintService<S>
Expand All @@ -33,23 +34,23 @@ where
}

fn call(&mut self, req: R) -> Self::Future {
println!("Hi {}", self.name);
println!("Hi {}", self.id.absolute());
self.inner.call(req)
}
}

/// A [`Layer`] which constructs the [`PrintService`].
#[derive(Debug)]
pub struct PrintLayer {
name: &'static str,
id: ShapeId,
}
impl<S> Layer<S> for PrintLayer {
type Service = PrintService<S>;

fn layer(&self, service: S) -> Self::Service {
PrintService {
inner: service,
name: self.name,
id: self.id.clone(),
}
}
}
Expand All @@ -66,7 +67,7 @@ where
type Layer = Stack<L, PrintLayer>;

fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
input.layer(PrintLayer { name: Op::NAME })
input.layer(PrintLayer { id: Op::NAME })
}
}

Expand Down
50 changes: 21 additions & 29 deletions rust-runtime/aws-smithy-http-server/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,28 @@
//!
//! [extensions]: https://docs.rs/http/latest/http/struct.Extensions.html

use std::{fmt, future::Future, ops::Deref, pin::Pin, task::Context, task::Poll};
use std::{fmt, fmt::Debug, future::Future, ops::Deref, pin::Pin, task::Context, task::Poll};
use std::fmt::{Display, Formatter};
use std::hash::Hash;

use futures_util::ready;
use futures_util::TryFuture;
use thiserror::Error;
use tower::{layer::util::Stack, Layer, Service};
use crate::extension;

use crate::operation::{Operation, OperationShape};
use crate::plugin::{plugin_from_operation_name_fn, OperationNameFn, Plugin, PluginPipeline, PluginStack};
use crate::shape_id::ShapeId;

pub use crate::request::extension::{Extension, MissingExtension};

/// Extension type used to store information about Smithy operations in HTTP responses.
/// This extension type is inserted, via the [`OperationExtensionPlugin`], whenever it has been correctly determined
/// that the request should be routed to a particular operation. The operation handler might not even get invoked
/// because the request fails to deserialize into the modeled operation input.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct OperationExtension {
absolute: &'static str,

namespace: &'static str,
name: &'static str,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct OperationExtension(pub ShapeId);

/// An error occurred when parsing an absolute operation shape ID.
#[derive(Debug, Clone, Error, PartialEq, Eq)]
Expand All @@ -53,31 +52,25 @@ pub enum ParseError {

#[allow(deprecated)]
impl OperationExtension {
82marbag marked this conversation as resolved.
Show resolved Hide resolved
/// Creates a new [`OperationExtension`] from the absolute shape ID.
pub fn new(absolute_operation_id: &'static str) -> Result<Self, ParseError> {
let (namespace, name) = absolute_operation_id
.rsplit_once('#')
.ok_or(ParseError::MissingNamespace)?;
Ok(Self {
absolute: absolute_operation_id,
namespace,
name,
})
}

/// Returns the Smithy model namespace.
pub fn namespace(&self) -> &'static str {
self.namespace
self.0.namespace()
}

/// Returns the Smithy operation name.
pub fn name(&self) -> &'static str {
self.name
self.0.name()
}

/// Returns the absolute operation shape ID.
pub fn absolute(&self) -> &'static str {
self.absolute
self.0.absolute()
}
}

impl Display for OperationExtension {
82marbag marked this conversation as resolved.
Show resolved Hide resolved
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
self.0.fmt(f)
}
}

Expand Down Expand Up @@ -154,7 +147,7 @@ impl<S> Layer<S> for OperationExtensionLayer {
}

/// A [`Plugin`] which applies [`OperationExtensionLayer`] to every operation.
pub struct OperationExtensionPlugin(OperationNameFn<fn(&'static str) -> OperationExtensionLayer>);
pub struct OperationExtensionPlugin(OperationNameFn<fn(ShapeId) -> OperationExtensionLayer>);

impl fmt::Debug for OperationExtensionPlugin {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -170,7 +163,7 @@ where
type Layer = Stack<L, OperationExtensionLayer>;

fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
<OperationNameFn<fn(&'static str) -> OperationExtensionLayer> as Plugin<P, Op, S, L>>::map(&self.0, input)
<OperationNameFn<fn(ShapeId) -> OperationExtensionLayer> as Plugin<P, Op, S, L>>::map(&self.0, input)
}
}

Expand All @@ -184,9 +177,8 @@ pub trait OperationExtensionExt<P> {

impl<P> OperationExtensionExt<P> for PluginPipeline<P> {
fn insert_operation_extension(self) -> PluginPipeline<PluginStack<OperationExtensionPlugin, P>> {
let plugin = OperationExtensionPlugin(plugin_from_operation_name_fn(|name| {
let operation_extension = OperationExtension::new(name).expect("Operation name is malformed, this should never happen. Please file an issue against https://github.com/awslabs/smithy-rs");
OperationExtensionLayer(operation_extension)
let plugin = OperationExtensionPlugin(plugin_from_operation_name_fn(|shape_id| {
OperationExtensionLayer(extension::OperationExtension(shape_id.clone()))
}));
self.push(plugin)
}
Expand Down Expand Up @@ -281,7 +273,7 @@ mod tests {
let svc = service_fn(|_: http::Request<()>| async { Ok::<_, ()>(http::Response::new(())) });
let svc = layer.layer(svc);

// Check for `OperationExtension`.
// Check for `OperationId`.
82marbag marked this conversation as resolved.
Show resolved Hide resolved
let response = svc.oneshot(http::Request::new(())).await.unwrap();
let expected = OperationExtension::new(DummyOp::NAME).unwrap();
let actual = response.extensions().get::<OperationExtension>().unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@

use tower::Layer;

use crate::shape_id::ShapeId;

use super::{InstrumentOperation, MakeIdentity};

/// A [`Layer`] used to apply [`InstrumentOperation`].
#[derive(Debug)]
pub struct InstrumentLayer<RequestMakeFmt = MakeIdentity, ResponseMakeFmt = MakeIdentity> {
operation_name: &'static str,
operation_id: ShapeId,
make_request: RequestMakeFmt,
make_response: ResponseMakeFmt,
}

impl InstrumentLayer {
/// Constructs a new [`InstrumentLayer`] with no data redacted.
pub fn new(operation_name: &'static str) -> Self {
pub fn new(operation_id: ShapeId) -> Self {
Self {
operation_name,
operation_id,
make_request: MakeIdentity,
make_response: MakeIdentity,
}
Expand All @@ -32,7 +34,7 @@ impl<RequestMakeFmt, ResponseMakeFmt> InstrumentLayer<RequestMakeFmt, ResponseMa
/// The argument is typically [`RequestFmt`](super::sensitivity::RequestFmt).
pub fn request_fmt<R>(self, make_request: R) -> InstrumentLayer<R, ResponseMakeFmt> {
InstrumentLayer {
operation_name: self.operation_name,
operation_id: self.operation_id,
make_request,
make_response: self.make_response,
}
Expand All @@ -43,7 +45,7 @@ impl<RequestMakeFmt, ResponseMakeFmt> InstrumentLayer<RequestMakeFmt, ResponseMa
/// The argument is typically [`ResponseFmt`](super::sensitivity::ResponseFmt).
pub fn response_fmt<R>(self, make_response: R) -> InstrumentLayer<RequestMakeFmt, R> {
InstrumentLayer {
operation_name: self.operation_name,
operation_id: self.operation_id,
make_request: self.make_request,
make_response,
}
Expand All @@ -58,7 +60,7 @@ where
type Service = InstrumentOperation<S, RequestMakeFmt, ResponseMakeFmt>;

fn layer(&self, service: S) -> Self::Service {
InstrumentOperation::new(service, self.operation_name)
InstrumentOperation::new(service, self.operation_id.clone())
.request_fmt(self.make_request.clone())
.response_fmt(self.make_response.clone())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ where
type Layer = Stack<L, InstrumentLayer<Op::RequestFmt, Op::ResponseFmt>>;

fn map(&self, operation: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
let layer = InstrumentLayer::new(Op::NAME)
let operation_id = Op::NAME;
let layer = InstrumentLayer::new(operation_id)
.request_fmt(Op::request_fmt())
.response_fmt(Op::response_fmt());
operation.layer(layer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use http::{HeaderMap, Request, Response, StatusCode, Uri};
use tower::Service;
use tracing::{debug, debug_span, instrument::Instrumented, Instrument};

use crate::shape_id::ShapeId;

use super::{MakeDebug, MakeDisplay, MakeIdentity};

pin_project_lite::pin_project! {
Expand Down Expand Up @@ -103,17 +105,17 @@ where
#[derive(Debug, Clone)]
pub struct InstrumentOperation<S, RequestMakeFmt = MakeIdentity, ResponseMakeFmt = MakeIdentity> {
inner: S,
operation_name: &'static str,
operation_id: ShapeId,
make_request: RequestMakeFmt,
make_response: ResponseMakeFmt,
}

impl<S> InstrumentOperation<S> {
/// Constructs a new [`InstrumentOperation`] with no data redacted.
pub fn new(inner: S, operation_name: &'static str) -> Self {
pub fn new(inner: S, operation_id: ShapeId) -> Self {
Self {
inner,
operation_name,
operation_id,
make_request: MakeIdentity,
make_response: MakeIdentity,
}
Expand All @@ -127,7 +129,7 @@ impl<S, RequestMakeFmt, ResponseMakeFmt> InstrumentOperation<S, RequestMakeFmt,
pub fn request_fmt<R>(self, make_request: R) -> InstrumentOperation<S, R, ResponseMakeFmt> {
InstrumentOperation {
inner: self.inner,
operation_name: self.operation_name,
operation_id: self.operation_id,
make_request,
make_response: self.make_response,
}
Expand All @@ -139,7 +141,7 @@ impl<S, RequestMakeFmt, ResponseMakeFmt> InstrumentOperation<S, RequestMakeFmt,
pub fn response_fmt<R>(self, make_response: R) -> InstrumentOperation<S, RequestMakeFmt, R> {
InstrumentOperation {
inner: self.inner,
operation_name: self.operation_name,
operation_id: self.operation_id,
make_request: self.make_request,
make_response,
}
Expand Down Expand Up @@ -170,7 +172,7 @@ where
let span = {
let headers = self.make_request.make_debug(request.headers());
let uri = self.make_request.make_display(request.uri());
debug_span!("request", operation = %self.operation_name, method = %request.method(), %uri, ?headers)
debug_span!("request", operation = %self.operation_id.absolute(), method = %request.method(), %uri, ?headers)
};

InstrumentedFuture {
Expand Down
1 change: 1 addition & 0 deletions rust-runtime/aws-smithy-http-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub mod response;
pub mod routing;
#[doc(hidden)]
pub mod runtime_error;
pub mod shape_id;

#[doc(inline)]
pub(crate) use self::error::Error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
* SPDX-License-Identifier: Apache-2.0
*/

use crate::shape_id::ShapeId;
use super::{Handler, IntoService, Normalize, Operation, OperationService};

/// Models the [Smithy Operation shape].
///
/// [Smithy Operation shape]: https://awslabs.github.io/smithy/1.0/spec/core/model.html#operation
pub trait OperationShape {
/// The name of the operation.
const NAME: &'static str;
const NAME: ShapeId;

/// The operation input.
type Input;
Expand Down
Loading