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
8 changes: 8 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,11 @@ message = "Avoid extending IMDS credentials' expiry unconditionally, which may i
references = ["smithy-rs#2687", "smithy-rs#2694"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"

[[smithy-rs]]
message = """`ShapeId` is the new structure used to represent a shape, with its absolute name, namespace and name.
`OperationExtension`'s members are replaced by the `ShapeId` and operations' names are now replced by a `ShapeId`.
"""
author = "82marbag"
references = ["smithy-rs#2678"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
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 ServerServiceGenerator(
}

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 ServerServiceGenerator(
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 ServerServiceGenerator(

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

Expand Down
11 changes: 6 additions & 5 deletions examples/pokemon-service/src/plugin.rs
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
80 changes: 23 additions & 57 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,27 @@
//!
//! [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::hash::Hash;
use std::{fmt, fmt::Debug, future::Future, ops::Deref, pin::Pin, task::Context, task::Poll};

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

use crate::operation::{Operation, OperationShape};
use crate::plugin::{plugin_from_operation_name_fn, OperationNameFn, Plugin, PluginPipeline, PluginStack};
use crate::plugin::{plugin_from_operation_id_fn, OperationIdFn, 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 @@ -51,36 +49,6 @@ pub enum ParseError {
MissingNamespace,
}

#[allow(deprecated)]
impl OperationExtension {
/// 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
}

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

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

pin_project_lite::pin_project! {
/// The [`Service::Future`] of [`OperationExtensionService`] - inserts an [`OperationExtension`] into the
/// [`http::Response]`.
Expand Down Expand Up @@ -154,7 +122,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(OperationIdFn<fn(ShapeId) -> OperationExtensionLayer>);

impl fmt::Debug for OperationExtensionPlugin {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -170,7 +138,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)
<OperationIdFn<fn(ShapeId) -> OperationExtensionLayer> as Plugin<P, Op, S, L>>::map(&self.0, input)
}
}

Expand All @@ -184,9 +152,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_id_fn(|shape_id| {
OperationExtensionLayer(extension::OperationExtension(shape_id))
}));
self.push(plugin)
}
Expand Down Expand Up @@ -243,28 +210,27 @@ mod tests {
#[test]
fn ext_accept() {
let value = "com.amazonaws.ebs#CompleteSnapshot";
let ext = OperationExtension::new(value).unwrap();
let ext = ShapeId::new(
"com.amazonaws.ebs#CompleteSnapshot",
"com.amazonaws.ebs",
"CompleteSnapshot",
);

assert_eq!(ext.absolute(), value);
assert_eq!(ext.namespace(), "com.amazonaws.ebs");
assert_eq!(ext.name(), "CompleteSnapshot");
}

#[test]
fn ext_reject() {
let value = "CompleteSnapshot";
assert_eq!(
OperationExtension::new(value).unwrap_err(),
ParseError::MissingNamespace
)
}

#[tokio::test]
async fn plugin() {
struct DummyOp;

impl OperationShape for DummyOp {
const NAME: &'static str = "com.amazonaws.ebs#CompleteSnapshot";
const NAME: ShapeId = ShapeId::new(
"com.amazonaws.ebs#CompleteSnapshot",
"com.amazonaws.ebs",
"CompleteSnapshot",
);

type Input = ();
type Output = ();
Expand All @@ -283,8 +249,8 @@ mod tests {

// Check for `OperationExtension`.
let response = svc.oneshot(http::Request::new(())).await.unwrap();
let expected = OperationExtension::new(DummyOp::NAME).unwrap();
let expected = DummyOp::NAME;
let actual = response.extensions().get::<OperationExtension>().unwrap();
assert_eq!(*actual, expected);
assert_eq!(actual.0, expected);
}
}
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 @@ -13,13 +13,15 @@
//! ```
//! # use std::convert::Infallible;
//! # use aws_smithy_http_server::instrumentation::{*, sensitivity::{*, headers::*, uri::*}};
//! # use aws_smithy_http_server::shape_id::ShapeId;
//! # use http::{Request, Response};
//! # use tower::{util::service_fn, Service};
//! # async fn service(request: Request<()>) -> Result<Response<()>, Infallible> {
//! # Ok(Response::new(()))
//! # }
//! # async fn example() {
//! # let service = service_fn(service);
//! # const NAME: ShapeId = ShapeId::new("namespace#foo-operation", "namespace", "foo-operation");
//! let request = Request::get("http://localhost/a/b/c/d?bar=hidden")
//! .header("header-name-a", "hidden")
//! .body(())
Expand Down Expand Up @@ -47,7 +49,7 @@
//! }
//! })
//! .status_code();
//! let mut service = InstrumentOperation::new(service, "foo-operation")
//! let mut service = InstrumentOperation::new(service, NAME)
//! .request_fmt(request_fmt)
//! .response_fmt(response_fmt);
//!
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
Loading