Revert "OperationShape::NAME as ShapeId (#2678)" (#2698)

This reverts #2678.

It is a breaking change and it will be included in the next release.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

CHANGELOG.next.toml

@@ -29,11 +29,3 @@ 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 }

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

@@ -13,7 +13,6 @@ 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
 
@@ -50,13 +49,12 @@ 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: #{SmithyHttpServer}::shape_id::ShapeId = #{SmithyHttpServer}::shape_id::ShapeId::new(${operationIdAbsolute.dq()}, ${operationId.namespace.dq()}, ${operationId.name.dq()});
+                const NAME: &'static str = "${operationId.toString().replace("#", "##")}";
 
                 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 {
-        rustTemplate(
+        rust(
             """
             /// 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<#{SmithyHttpServer}::shape_id::ShapeId, &'static str>,
+                operation_names2setter_methods: std::collections::HashMap<&'static str, &'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.absolute())?;
+                        writeln!(f, "- {}", operation_name)?;
                     }
 
                     writeln!(f, "\nUse the dedicated methods on `$builderName` to register the missing handlers:")?;
@@ -508,7 +508,6 @@ class ServerServiceGenerator(
 
             impl std::error::Error for MissingOperationsError {}
             """,
-            *codegenScope,
         )
     }
 

examples/pokemon-service/src/plugin.rs

@@ -8,7 +8,6 @@
 use aws_smithy_http_server::{
     operation::{Operation, OperationShape},
     plugin::{Plugin, PluginPipeline, PluginStack},
-    shape_id::ShapeId,
 };
 use tower::{layer::util::Stack, Layer, Service};
 
@@ -18,7 +17,7 @@ use std::task::{Context, Poll};
 #[derive(Clone, Debug)]
 pub struct PrintService<S> {
     inner: S,
-    id: ShapeId,
+    name: &'static str,
 }
 
 impl<R, S> Service<R> for PrintService<S>
@@ -34,23 +33,23 @@ where
     }
 
     fn call(&mut self, req: R) -> Self::Future {
-        println!("Hi {}", self.id.absolute());
+        println!("Hi {}", self.name);
         self.inner.call(req)
     }
 }
 
 /// A [`Layer`] which constructs the [`PrintService`].
 #[derive(Debug)]
 pub struct PrintLayer {
-    id: ShapeId,
+    name: &'static str,
 }
 impl<S> Layer<S> for PrintLayer {
     type Service = PrintService<S>;
 
     fn layer(&self, service: S) -> Self::Service {
         PrintService {
             inner: service,
-            id: self.id.clone(),
+            name: self.name,
         }
     }
 }
@@ -67,7 +66,7 @@ where
     type Layer = Stack<L, PrintLayer>;
 
     fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
-        input.layer(PrintLayer { id: Op::NAME })
+        input.layer(PrintLayer { name: Op::NAME })
     }
 }
 

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

@@ -19,27 +19,29 @@
 //!
 //! [extensions]: https://docs.rs/http/latest/http/struct.Extensions.html
 
-use std::hash::Hash;
-use std::{fmt, fmt::Debug, future::Future, ops::Deref, pin::Pin, task::Context, task::Poll};
+use std::{fmt, 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_id_fn, OperationIdFn, Plugin, PluginPipeline, PluginStack};
-use crate::shape_id::ShapeId;
+use crate::plugin::{plugin_from_operation_name_fn, OperationNameFn, Plugin, PluginPipeline, PluginStack};
 
 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, Hash)]
-pub struct OperationExtension(pub ShapeId);
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct OperationExtension {
+    absolute: &'static str,
+
+    namespace: &'static str,
+    name: &'static str,
+}
 
 /// An error occurred when parsing an absolute operation shape ID.
 #[derive(Debug, Clone, Error, PartialEq, Eq)]
@@ -49,6 +51,36 @@ 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]`.
@@ -122,7 +154,7 @@ impl<S> Layer<S> for OperationExtensionLayer {
 }
 
 /// A [`Plugin`] which applies [`OperationExtensionLayer`] to every operation.
-pub struct OperationExtensionPlugin(OperationIdFn<fn(ShapeId) -> OperationExtensionLayer>);
+pub struct OperationExtensionPlugin(OperationNameFn<fn(&'static str) -> OperationExtensionLayer>);
 
 impl fmt::Debug for OperationExtensionPlugin {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -138,7 +170,7 @@ where
     type Layer = Stack<L, OperationExtensionLayer>;
 
     fn map(&self, input: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
-        <OperationIdFn<fn(ShapeId) -> OperationExtensionLayer> as Plugin<P, Op, S, L>>::map(&self.0, input)
+        <OperationNameFn<fn(&'static str) -> OperationExtensionLayer> as Plugin<P, Op, S, L>>::map(&self.0, input)
     }
 }
 
@@ -152,8 +184,9 @@ 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_id_fn(|shape_id| {
-            OperationExtensionLayer(extension::OperationExtension(shape_id))
+        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)
         }));
         self.push(plugin)
     }
@@ -210,27 +243,28 @@ mod tests {
     #[test]
     fn ext_accept() {
         let value = "com.amazonaws.ebs#CompleteSnapshot";
-        let ext = ShapeId::new(
-            "com.amazonaws.ebs#CompleteSnapshot",
-            "com.amazonaws.ebs",
-            "CompleteSnapshot",
-        );
+        let ext = OperationExtension::new(value).unwrap();
 
         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: ShapeId = ShapeId::new(
-                "com.amazonaws.ebs#CompleteSnapshot",
-                "com.amazonaws.ebs",
-                "CompleteSnapshot",
-            );
+            const NAME: &'static str = "com.amazonaws.ebs#CompleteSnapshot";
 
             type Input = ();
             type Output = ();
@@ -249,8 +283,8 @@ mod tests {
 
         // Check for `OperationExtension`.
         let response = svc.oneshot(http::Request::new(())).await.unwrap();
-        let expected = DummyOp::NAME;
+        let expected = OperationExtension::new(DummyOp::NAME).unwrap();
         let actual = response.extensions().get::<OperationExtension>().unwrap();
-        assert_eq!(actual.0, expected);
+        assert_eq!(*actual, expected);
     }
 }

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

@@ -5,23 +5,21 @@
 
 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_id: ShapeId,
+    operation_name: &'static str,
     make_request: RequestMakeFmt,
     make_response: ResponseMakeFmt,
 }
 
 impl InstrumentLayer {
     /// Constructs a new [`InstrumentLayer`] with no data redacted.
-    pub fn new(operation_id: ShapeId) -> Self {
+    pub fn new(operation_name: &'static str) -> Self {
         Self {
-            operation_id,
+            operation_name,
             make_request: MakeIdentity,
             make_response: MakeIdentity,
         }
@@ -34,7 +32,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_id: self.operation_id,
+            operation_name: self.operation_name,
             make_request,
             make_response: self.make_response,
         }
@@ -45,7 +43,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_id: self.operation_id,
+            operation_name: self.operation_name,
             make_request: self.make_request,
             make_response,
         }
@@ -60,7 +58,7 @@ where
     type Service = InstrumentOperation<S, RequestMakeFmt, ResponseMakeFmt>;
 
     fn layer(&self, service: S) -> Self::Service {
-        InstrumentOperation::new(service, self.operation_id.clone())
+        InstrumentOperation::new(service, self.operation_name)
             .request_fmt(self.make_request.clone())
             .response_fmt(self.make_response.clone())
     }

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

@@ -13,15 +13,13 @@
 //! ```
 //! # 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(())
@@ -49,7 +47,7 @@
 //!         }
 //!     })
 //!     .status_code();
-//! let mut service = InstrumentOperation::new(service, NAME)
+//! let mut service = InstrumentOperation::new(service, "foo-operation")
 //!     .request_fmt(request_fmt)
 //!     .response_fmt(response_fmt);
 //!

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

@@ -26,8 +26,7 @@ where
     type Layer = Stack<L, InstrumentLayer<Op::RequestFmt, Op::ResponseFmt>>;
 
     fn map(&self, operation: Operation<S, L>) -> Operation<Self::Service, Self::Layer> {
-        let operation_id = Op::NAME;
-        let layer = InstrumentLayer::new(operation_id)
+        let layer = InstrumentLayer::new(Op::NAME)
             .request_fmt(Op::request_fmt())
             .response_fmt(Op::response_fmt());
         operation.layer(layer)