OperationShape::NAME as ShapeId (#2678) (#2717)

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>
Co-authored-by: Harry Barber <106155934+hlbarber@users.noreply.github.com>

Author: 82marbag

CHANGELOG.next.toml

@@ -133,3 +133,32 @@ message = "A newtype wrapper `SharedAsyncSleep` has been introduced and occurren
 references = ["smithy-rs#2742"]
 meta = { "breaking" = true, "tada" = false, "bug" = false }
 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`.
+
+Before you had an operation and an absolute name as its `NAME` member. You could apply a plugin only to some selected operation:
+
+```
+filter_by_operation_name(plugin, |name| name != Op::NAME);
+```
+
+Your new filter selects on an operation's absolute name, namespace or name.
+
+```
+filter_by_operation_id(plugin, |id| id.name() != Op::NAME.name());
+```
+
+The above filter is applied to an operation's name, the one you use to specify the operation in the Smithy model.
+
+You can filter all operations in a namespace or absolute name:
+
+```
+filter_by_operation_id(plugin, |id| id.namespace() != "namespace");
+filter_by_operation_id(plugin, |id| id.absolute() != "namespace#name");
+```
+"""
+author = "82marbag"
+references = ["smithy-rs#2678"]
+meta = { "breaking" = true, "tada" = false, "bug" = false }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationGenerator.kt

@@ -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
 
@@ -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;

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt

@@ -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 {
@@ -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:")?;
@@ -508,6 +508,7 @@ class ServerServiceGenerator(
 
             impl std::error::Error for MissingOperationsError {}
             """,
+            *codegenScope,
         )
     }
 

examples/pokemon-service/src/plugin.rs

@@ -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};
 
@@ -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>
@@ -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(),
         }
     }
 }
@@ -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 })
     }
 }
 

rust-runtime/aws-smithy-http-server/src/extension.rs

@@ -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)]
@@ -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]`.
@@ -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 {
@@ -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)
     }
 }
 
@@ -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)
     }
@@ -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 = ();
@@ -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);
     }
 }

rust-runtime/aws-smithy-http-server/src/instrumentation/layer.rs

@@ -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,
         }
@@ -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,
         }
@@ -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,
         }
@@ -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())
     }

rust-runtime/aws-smithy-http-server/src/instrumentation/mod.rs

@@ -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(())
@@ -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);
 //!

rust-runtime/aws-smithy-http-server/src/instrumentation/plugin.rs

@@ -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)