`aws-smithy-http-server`: implement basic URI pattern conflict disambiguation (#1036) (5a07828e) · Commits · Public Repositories / Smithy Rust

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

+13 −16

Original line number	Diff line number	Diff line
		@@ -14,6 +14,7 @@ import software.amazon.smithy.rust.codegen.rustlang.rust
		import software.amazon.smithy.rust.codegen.rustlang.rustBlock
		import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate
		import software.amazon.smithy.rust.codegen.rustlang.rustTemplate
		import software.amazon.smithy.rust.codegen.server.smithy.ServerCargoDependency
		import software.amazon.smithy.rust.codegen.server.smithy.ServerRuntimeType
		import software.amazon.smithy.rust.codegen.smithy.CodegenContext
		import software.amazon.smithy.rust.codegen.smithy.RuntimeType
		@@ -29,9 +30,6 @@ class ServerOperationRegistryGenerator(
		private val httpBindingResolver: HttpBindingResolver,
		private val operations: List<OperationShape>,
		) {
		private val serverCrate = "aws_smithy_http_server"
		private val service = codegenContext.serviceShape
		private val model = codegenContext.model
		private val symbolProvider = codegenContext.symbolProvider
		private val operationNames = operations.map { symbolProvider.toSymbol(it).name.toSnakeCase() }
		private val runtimeConfig = codegenContext.runtimeConfig
		@@ -39,6 +37,7 @@ class ServerOperationRegistryGenerator(
		"Router" to ServerRuntimeType.Router(runtimeConfig),
		"SmithyHttpServer" to CargoDependency.SmithyHttpServer(runtimeConfig).asType(),
		"ServerOperationHandler" to ServerRuntimeType.serverOperationHandler(runtimeConfig),
		"Tower" to ServerCargoDependency.Tower.asType(),
		"Phantom" to ServerRuntimeType.Phantom,
		"StdError" to RuntimeType.StdError
		)
		@@ -216,18 +215,17 @@ class ServerOperationRegistryGenerator(
		) {
		rustBlock("fn from(registry: $operationRegistryNameWithArguments) -> Self") {
		val requestSpecsVarNames = operationNames.map { "${it}_request_spec" }
		val routes = requestSpecsVarNames.zip(operationNames) { requestSpecVarName, operationName ->
		".route($requestSpecVarName, #{ServerOperationHandler}::operation(registry.$operationName))"
		}.joinToString(separator = "\n")

		val requestSpecs = requestSpecsVarNames.zip(operations) { requestSpecVarName, operation ->
		"let $requestSpecVarName = ${operation.requestSpec()};"
		}.joinToString(separator = "\n")
		val towerServices = requestSpecsVarNames.zip(operationNames) { requestSpecVarName, operationName ->
		"(#{Tower}::util::BoxCloneService::new(#{ServerOperationHandler}::operation(registry.$operationName)), $requestSpecVarName)"
		}.joinToString(prefix = "vec![", separator = ",\n", postfix = "]")

		rustTemplate(
		"""
		$requestSpecs
		#{Router}::new()
		$routes
		#{Router}::from_box_clone_service_iter($towerServices)
		""".trimIndent(),
		*codegenScope
		)
		@@ -267,13 +265,12 @@ class ServerOperationRegistryGenerator(
		return """
		$namespace::RequestSpec::new(
		http::Method::${httpTrait.method},
		$namespace::UriSpec {
		host_prefix: None,
		path_and_query: $namespace::PathAndQuerySpec {
		path_segments: $namespace::PathSpec::from_vector_unchecked(vec![${pathSegments.joinToString()}]),
		query_segments: $namespace::QuerySpec::from_vector_unchecked(vec![${querySegments.joinToString()}])
		}
		}
		$namespace::UriSpec::new(
		$namespace::PathAndQuerySpec::new(
		$namespace::PathSpec::from_vector_unchecked(vec![${pathSegments.joinToString()}]),
		$namespace::QuerySpec::from_vector_unchecked(vec![${querySegments.joinToString()}])
		)
		)
		)
		""".trimIndent()
		}

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

+108 −26

Original line number	Diff line number	Diff line
		@@ -20,9 +20,12 @@ use tower::util::ServiceExt;
		use tower::{Service, ServiceBuilder};
		use tower_http::map_response_body::MapResponseBodyLayer;

		pub mod future;
		mod future;
		mod into_make_service;

		#[doc(hidden)]
		pub mod request_spec;

		mod route;

		pub use self::{into_make_service::IntoMakeService, route::Route};
		@@ -54,7 +57,9 @@ where
		B: Send + 'static,
		{
		fn default() -> Self {
		Self::new()
		Self {
		routes: Default::default(),
		}
		}
		}

		@@ -62,26 +67,30 @@ impl<B> Router<B>
		where
		B: Send + 'static,
		{
		/// Create a new `Router`.
		/// Create a new `Router` from a vector of pairs of request specs and services.
		///
		/// Unless you add additional routes this will respond to `404 Not Found` to
		/// all requests.
		/// If the vector is empty the router will respond `404 Not Found` to all requests.
		#[doc(hidden)]
		pub fn new() -> Self {
		Self {
		routes: Default::default(),
		}
		}

		/// Add a route to the router.
		#[doc(hidden)]
		pub fn route<T>(mut self, request_spec: RequestSpec, svc: T) -> Self
		pub fn from_box_clone_service_iter<T>(routes: T) -> Self
		where
		T: Service<Request<B>, Response = Response<BoxBody>, Error = Infallible> + Clone + Send + 'static,
		T::Future: Send + 'static,
		T: IntoIterator<
		Item = (
		tower::util::BoxCloneService<Request<B>, Response<BoxBody>, Infallible>,
		RequestSpec,
		),
		>,
		{
		self.routes.push((Route::new(svc), request_spec));
		self
		let mut routes: Vec<(Route<B>, RequestSpec)> = routes
		.into_iter()
		.map(\|(svc, request_spec)\| (Route::from_box_clone_service(svc), request_spec))
		.collect();

		// Sort them once by specifity, with the more specific routes sorted before the less
		// specific ones, so that when routing a request we can simply iterate through the routes
		// and pick the first one that matches.
		routes.sort_by_key(\|(_route, request_spec)\| std::cmp::Reverse(request_spec.rank()));

		Self { routes }
		}

		/// Convert this router into a [`MakeService`], that is a [`Service`] whose
		@@ -225,7 +234,7 @@ mod tests {
		PathSegment::Label,
		PathSegment::Label,
		],
		vec![],
		Vec::new(),
		),
		"A",
		),
		@@ -237,14 +246,14 @@ mod tests {
		PathSegment::Greedy,
		PathSegment::Literal(String::from("z")),
		],
		vec![],
		Vec::new(),
		),
		"MiddleGreedy",
		),
		(
		RequestSpec::from_parts(
		Method::DELETE,
		vec![],
		Vec::new(),
		vec![
		QuerySegment::KeyValue(String::from("foo"), String::from("bar")),
		QuerySegment::Key(String::from("baz")),
		@@ -262,11 +271,12 @@ mod tests {
		),
		];

		let mut router = Router::new();
		for (spec, svc_name) in request_specs {
		let svc = NamedEchoUriService(String::from(svc_name));
		router = router.route(spec, svc.clone());
		}
		let mut router = Router::from_box_clone_service_iter(request_specs.into_iter().map(\|(spec, svc_name)\| {
		(
		tower::util::BoxCloneService::new(NamedEchoUriService(String::from(svc_name))),
		spec,
		)
		}));

		let hits = vec![
		("A", Method::GET, "/a/b/c"),
		@@ -312,4 +322,76 @@ mod tests {
		assert_eq!(StatusCode::NOT_FOUND, res.status());
		}
		}

		#[tokio::test]
		async fn basic_pattern_conflict_avoidance() {
		let request_specs: Vec<(RequestSpec, &str)> = vec![
		(
		RequestSpec::from_parts(
		Method::GET,
		vec![PathSegment::Literal(String::from("a")), PathSegment::Label],
		Vec::new(),
		),
		"A1",
		),
		(
		RequestSpec::from_parts(
		Method::GET,
		vec![
		PathSegment::Literal(String::from("a")),
		PathSegment::Label,
		PathSegment::Literal(String::from("a")),
		],
		Vec::new(),
		),
		"A2",
		),
		(
		RequestSpec::from_parts(
		Method::GET,
		vec![PathSegment::Literal(String::from("b")), PathSegment::Greedy],
		Vec::new(),
		),
		"B1",
		),
		(
		RequestSpec::from_parts(
		Method::GET,
		vec![PathSegment::Literal(String::from("b")), PathSegment::Greedy],
		vec![QuerySegment::Key(String::from("q"))],
		),
		"B2",
		),
		(
		RequestSpec::from_parts(Method::GET, Vec::new(), Vec::new()),
		"ListBuckets",
		),
		(
		RequestSpec::from_parts(Method::GET, vec![PathSegment::Label], Vec::new()),
		"ListObjects",
		),
		];

		let mut router = Router::from_box_clone_service_iter(request_specs.into_iter().map(\|(spec, svc_name)\| {
		(
		tower::util::BoxCloneService::new(NamedEchoUriService(String::from(svc_name))),
		spec,
		)
		}));

		let hits = vec![
		("A1", Method::GET, "/a/foo"),
		("A2", Method::GET, "/a/foo/a"),
		("B1", Method::GET, "/b/foo/bar/baz"),
		("B2", Method::GET, "/b/foo?q=baz"),
		("ListBuckets", Method::GET, "/"),
		("ListObjects", Method::GET, "/bucket"),
		];
		for (svc_name, method, uri) in &hits {
		let mut res = router.call(req(method, uri)).await.unwrap();
		let actual_body = get_body_as_str(&mut res).await;

		assert_eq!(format!("{} :: {}", svc_name, uri), actual_body);
		}
		}
		}

rust-runtime/aws-smithy-http-server/src/routing/request_spec.rs

+59 −5

Original line number	Diff line number	Diff line
		@@ -45,14 +45,35 @@ impl QuerySpec {

		#[derive(Debug, Clone, Default)]
		pub struct PathAndQuerySpec {
		pub path_segments: PathSpec,
		pub query_segments: QuerySpec,
		path_segments: PathSpec,
		query_segments: QuerySpec,
		}

		impl PathAndQuerySpec {
		pub fn new(path_segments: PathSpec, query_segments: QuerySpec) -> Self {
		PathAndQuerySpec {
		path_segments,
		query_segments,
		}
		}
		}

		#[derive(Debug, Clone)]
		pub struct UriSpec {
		pub host_prefix: Option<Vec<HostPrefixSegment>>,
		pub path_and_query: PathAndQuerySpec,
		host_prefix: Option<Vec<HostPrefixSegment>>,
		path_and_query: PathAndQuerySpec,
		}

		impl UriSpec {
		// TODO When we add support for the endpoint trait, this constructor will take in
		// a first argument `host_prefix`.
		// https://awslabs.github.io/smithy/1.0/spec/core/endpoint-traits.html#endpoint-trait
		pub fn new(path_and_query: PathAndQuerySpec) -> Self {
		UriSpec {
		host_prefix: None,
		path_and_query,
		}
		}
		}

		#[derive(Debug, Clone)]
		@@ -63,7 +84,7 @@ pub struct RequestSpec {
		}

		#[derive(Debug, PartialEq)]
		pub enum Match {
		pub(super) enum Match {
		/// The request matches the URI pattern spec.
		Yes,
		/// The request matches the URI pattern spec, but the wrong HTTP method was used. `405 Method
		@@ -107,6 +128,39 @@ impl RequestSpec {
		}
		}

		/// A measure of how "important" a `RequestSpec` is. The more specific a `RequestSpec` is, the
		/// higher it ranks in importance. Specificity is measured by the number of segments plus the
		/// number of query string literals in its URI pattern, so `/{Bucket}/{Key}?query` is more
		/// specific than `/{Bucket}/{Key}`, which is more specific than `/{Bucket}`, which is more
		/// specific than `/`.
		///
		/// This rank effectively induces a total order, but we don't implement as `Ord` for
		/// `RequestSpec` because it would appear in its public interface.
		///
		/// # Why do we need this?
		///
		/// Note that:
		/// 1. the Smithy spec does not define how servers should route incoming requests in the
		/// case of pattern conflicts; and
		/// 2. the Smithy spec even outright rejects conflicting patterns that can be easily
		/// disambiguated e.g. `/{a}` and `/{label}/b` cannot coexist.
		///
		/// We can't to anything about (2) since the Smithy CLI will refuse to build a model with those
		/// kind of conflicts. However, the Smithy CLI does allow _other_ conflicting patterns to
		/// coexist, e.g. `/` and `/{label}`. We therefore have to take a stance on (1), since if we
		/// route arbitrarily [we render basic usage
		/// impossible](https://github.com/awslabs/smithy-rs/issues/1009).
		/// So this ranking of routes implements some basic pattern conflict disambiguation with some
		/// common sense. It's also the same behavior that [the TypeScript sSDK is implementing].
		///
		/// TODO(https://github.com/awslabs/smithy/issues/1029#issuecomment-1002683552): Once Smithy
		/// updates the spec to define the behavior, update our implementation.
		///
		/// [the TypeScript sSDK is implementing]: https://github.com/awslabs/smithy-typescript/blob/d263078b81485a6a2013d243639c0c680343ff47/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts#L59.
		pub(super) fn rank(&self) -> usize {
		self.uri_spec.path_and_query.path_segments.0.len() + self.uri_spec.path_and_query.query_segments.0.len()
		}

		pub(super) fn matches<B>(&self, req: &Request<B>) -> Match {
		if let Some(_host_prefix) = &self.uri_spec.host_prefix {
		todo!("Look at host prefix");

rust-runtime/aws-smithy-http-server/src/routing/route.rs

+4 −0

Original line number	Diff line number	Diff line
		@@ -61,6 +61,10 @@ impl<B> Route<B> {
		service: BoxCloneService::new(svc),
		}
		}

		pub(super) fn from_box_clone_service(svc: BoxCloneService<Request<B>, Response<BoxBody>, Infallible>) -> Self {
		Self { service: svc }
		}
		}

		impl<ReqBody> Clone for Route<ReqBody> {