Skip to content

Commit

Permalink
OperationShape::NAME as ShapeId (#2678)
Browse files Browse the repository at this point in the history
See #2634

## 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
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK 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._

---------

Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
  • Loading branch information
82marbag authored and david-perez committed May 18, 2023
1 parent ad37596 commit 3689772
Show file tree
Hide file tree
Showing 17 changed files with 184 additions and 125 deletions.
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);
}
}
14 changes: 8 additions & 6 deletions rust-runtime/aws-smithy-http-server/src/instrumentation/layer.rs
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

0 comments on commit 3689772

Please sign in to comment.