From ac95a5d1f033725639acc70be96e65188320aa02 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 30 Jun 2023 10:46:11 -0700 Subject: [PATCH 1/6] Migrate to a new SDK example workspace structure (#2811) ## Motivation and Context When the WebAssembly SDK example was added some months ago, we changed the build process to make each SDK example its own Cargo workspace. This allowed the `.cargo/config.toml` that changed the compiler target to work correctly. However, this has led to other issues: dependency compilation is no longer shared between examples which greatly increases the time it takes for that CI step to run, and now it is even running out of disk space on the GitHub Actions runners. This PR adds support for a new workspace layout where the [`aws-doc-sdk-examples`](https://github.com/awsdocs/aws-doc-sdk-examples) repo gets to decide how the workspaces are logically grouped. If a `Cargo.toml` file exists at the example root, then the build system assumes that the _old_ "one example, one workspace" layout should be used. If there is no root `Cargo.toml`, then it assumes the new layout should be used. The `sdk-versioner` tool had to be adapted to support more complex relative path resolution to make this work, and the `publisher fix-manifests` subcommand had to be fixed to ignore workspace-only `Cargo.toml` files. The build system in this PR needs to work for both the old and new examples layout so that the `sdk-sync` process will succeed. #2810 has been filed to track removing the old example layout at a later date. [aws-doc-sdk-examples#4997](https://github.com/awsdocs/aws-doc-sdk-examples/pull/4997) changes the workspace structure of the actual examples to the new one. ## Testing - [x] Generated a full SDK with the old example layout, manually examined the output, and spot checked that some examples compile - [x] Generated a full SDK with the new example layout, manually examined the output, and spot checked that some examples compile - [x] Examples pass in CI with the old example layout - [x] Examples pass in CI with the new example layout ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- aws/sdk/build.gradle.kts | 25 +- .../src/main/kotlin/aws/sdk/ServiceLoader.kt | 46 +++- tools/ci-build/publisher/src/package.rs | 65 +++-- .../src/subcommand/claim_crate_names.rs | 21 +- .../publisher/src/subcommand/fix_manifests.rs | 14 +- .../subcommand/generate_version_manifest.rs | 7 +- tools/ci-build/sdk-versioner/Cargo.lock | 7 + tools/ci-build/sdk-versioner/Cargo.toml | 1 + tools/ci-build/sdk-versioner/src/main.rs | 226 +++++++++++++++--- tools/ci-scripts/check-aws-sdk-examples | 2 +- tools/ci-scripts/generate-aws-sdk | 3 +- 11 files changed, 317 insertions(+), 100 deletions(-) diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index e90752cce33..d5225d66a9d 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import aws.sdk.AwsExamplesLayout import aws.sdk.AwsServices import aws.sdk.Membership import aws.sdk.discoverServices @@ -201,6 +202,7 @@ tasks.register("relocateExamples") { } into(outputDir) exclude("**/target") + exclude("**/rust-toolchain.toml") filter { line -> line.replace("build/aws-sdk/sdk/", "sdk/") } } } @@ -242,13 +244,22 @@ tasks.register("fixExampleManifests") { toolPath = sdkVersionerToolPath binaryName = "sdk-versioner" - arguments = listOf( - "use-path-and-version-dependencies", - "--isolate-crates", - "--sdk-path", "../../sdk", - "--versions-toml", outputDir.resolve("versions.toml").absolutePath, - outputDir.resolve("examples").absolutePath, - ) + arguments = when (AwsExamplesLayout.detect(project)) { + AwsExamplesLayout.Flat -> listOf( + "use-path-and-version-dependencies", + "--isolate-crates", + "--sdk-path", "../../sdk", + "--versions-toml", outputDir.resolve("versions.toml").absolutePath, + outputDir.resolve("examples").absolutePath, + ) + AwsExamplesLayout.Workspaces -> listOf( + "use-path-and-version-dependencies", + "--isolate-crates", + "--sdk-path", sdkOutputDir.absolutePath, + "--versions-toml", outputDir.resolve("versions.toml").absolutePath, + outputDir.resolve("examples").absolutePath, + ) + } outputs.dir(outputDir) dependsOn("relocateExamples", "generateVersionManifest") diff --git a/buildSrc/src/main/kotlin/aws/sdk/ServiceLoader.kt b/buildSrc/src/main/kotlin/aws/sdk/ServiceLoader.kt index c1f76be0c0c..189bce6a67c 100644 --- a/buildSrc/src/main/kotlin/aws/sdk/ServiceLoader.kt +++ b/buildSrc/src/main/kotlin/aws/sdk/ServiceLoader.kt @@ -19,6 +19,38 @@ data class RootTest( val manifestName: String, ) +// TODO(https://github.com/awslabs/smithy-rs/issues/2810): We can remove the `Flat` layout after the switch +// to `Workspaces` has been released. This can be checked by looking at the `examples/` directory in aws-sdk-rust's +// main branch. +// +// The `Flat` layout is retained for backwards compatibility so that the next release process can succeed. +enum class AwsExamplesLayout { + /** + * Directory layout for examples used prior to June 26, 2023, + * where each example was in the `rust_dev_preview/` root directory and + * was considered to be its own workspace. + * + * This layout had issues with CI in terms of time to compile and disk space required + * since the dependencies would get recompiled for every example. + */ + Flat, + + /** + * Current directory layout where there are a small number of workspaces + * rooted in `rust_dev_preview/`. + */ + Workspaces, + ; + + companion object { + fun detect(project: Project): AwsExamplesLayout = if (project.projectDir.resolve("examples/Cargo.toml").exists()) { + AwsExamplesLayout.Flat + } else { + AwsExamplesLayout.Workspaces + } + } +} + class AwsServices( private val project: Project, services: List, @@ -44,10 +76,16 @@ class AwsServices( } val examples: List by lazy { - project.projectDir.resolve("examples") - .listFiles { file -> !file.name.startsWith(".") }.orEmpty().toList() - .filter { file -> manifestCompatibleWithGeneratedServices(file) } - .map { "examples/${it.name}" } + val examplesRoot = project.projectDir.resolve("examples") + if (AwsExamplesLayout.detect(project) == AwsExamplesLayout.Flat) { + examplesRoot.listFiles { file -> !file.name.startsWith(".") }.orEmpty().toList() + .filter { file -> manifestCompatibleWithGeneratedServices(file) } + .map { "examples/${it.name}" } + } else { + examplesRoot.listFiles { file -> + !file.name.startsWith(".") && file.isDirectory() && file.resolve("Cargo.toml").exists() + }.orEmpty().toList().map { "examples/${it.name}" } + } } /** diff --git a/tools/ci-build/publisher/src/package.rs b/tools/ci-build/publisher/src/package.rs index 7156c9214b4..c0cd981b0be 100644 --- a/tools/ci-build/publisher/src/package.rs +++ b/tools/ci-build/publisher/src/package.rs @@ -145,8 +145,7 @@ pub async fn discover_and_validate_package_batches( fs: Fs, path: impl AsRef, ) -> Result<(Vec, PackageStats)> { - let manifest_paths = discover_package_manifests(path.as_ref().into()).await?; - let packages = read_packages(fs, manifest_paths) + let packages = discover_packages(fs, path.as_ref().into()) .await? .into_iter() .filter(|package| package.publish == Publish::Allowed) @@ -176,7 +175,7 @@ pub enum Error { /// Discovers all Cargo.toml files under the given path recursively #[async_recursion::async_recursion] -pub async fn discover_package_manifests(path: PathBuf) -> Result> { +pub async fn discover_manifests(path: PathBuf) -> Result> { let mut manifests = Vec::new(); let mut read_dir = fs::read_dir(&path).await?; while let Some(entry) = read_dir.next_entry().await? { @@ -185,14 +184,19 @@ pub async fn discover_package_manifests(path: PathBuf) -> Result> { let manifest_path = package_path.join("Cargo.toml"); if manifest_path.exists() { manifests.push(manifest_path); - } else { - manifests.extend(discover_package_manifests(package_path).await?.into_iter()); } + manifests.extend(discover_manifests(package_path).await?.into_iter()); } } Ok(manifests) } +/// Discovers and parses all Cargo.toml files that are packages (as opposed to being exclusively workspaces) +pub async fn discover_packages(fs: Fs, path: PathBuf) -> Result> { + let manifest_paths = discover_manifests(path).await?; + read_packages(fs, manifest_paths).await +} + /// Parses a semver version number and adds additional error context when parsing fails. pub fn parse_version(manifest_path: &Path, version: &str) -> Result { Version::parse(version) @@ -219,26 +223,33 @@ fn read_dependencies(path: &Path, dependencies: &DepsSet) -> Result Result { +/// Returns `Ok(None)` when the Cargo.toml is a workspace rather than a package +fn read_package(path: &Path, manifest_bytes: &[u8]) -> Result> { let manifest = Manifest::from_slice(manifest_bytes) .with_context(|| format!("failed to load package manifest for {:?}", path))?; - let package = manifest - .package - .ok_or_else(|| Error::InvalidManifest(path.into())) - .context("crate manifest doesn't have a `[package]` section")?; - let name = package.name; - let version = parse_version(path, &package.version)?; - let handle = PackageHandle { name, version }; - let publish = match package.publish { - cargo_toml::Publish::Flag(true) => Publish::Allowed, - _ => Publish::NotAllowed, - }; - - let mut local_dependencies = BTreeSet::new(); - local_dependencies.extend(read_dependencies(path, &manifest.dependencies)?.into_iter()); - local_dependencies.extend(read_dependencies(path, &manifest.dev_dependencies)?.into_iter()); - local_dependencies.extend(read_dependencies(path, &manifest.build_dependencies)?.into_iter()); - Ok(Package::new(handle, path, local_dependencies, publish)) + if let Some(package) = manifest.package { + let name = package.name; + let version = parse_version(path, &package.version)?; + let handle = PackageHandle { name, version }; + let publish = match package.publish { + cargo_toml::Publish::Flag(true) => Publish::Allowed, + _ => Publish::NotAllowed, + }; + + let mut local_dependencies = BTreeSet::new(); + local_dependencies.extend(read_dependencies(path, &manifest.dependencies)?.into_iter()); + local_dependencies.extend(read_dependencies(path, &manifest.dev_dependencies)?.into_iter()); + local_dependencies + .extend(read_dependencies(path, &manifest.build_dependencies)?.into_iter()); + Ok(Some(Package::new( + handle, + path, + local_dependencies, + publish, + ))) + } else { + Ok(None) + } } /// Validates that all of the publishable crates use consistent version numbers @@ -275,7 +286,9 @@ pub async fn read_packages(fs: Fs, manifest_paths: Vec) -> Result = fs.read_file(path).await?; - result.push(read_package(path, &contents)?); + if let Some(package) = read_package(path, &contents)? { + result.push(package); + } } Ok(result) } @@ -350,7 +363,9 @@ mod tests { "#; let path: PathBuf = "test/Cargo.toml".into(); - let package = read_package(&path, manifest).expect("parse success"); + let package = read_package(&path, manifest) + .expect("parse success") + .expect("is a package"); assert_eq!("test", package.handle.name); assert_eq!(version("1.2.0-preview"), package.handle.version); diff --git a/tools/ci-build/publisher/src/subcommand/claim_crate_names.rs b/tools/ci-build/publisher/src/subcommand/claim_crate_names.rs index 72a3285ddb0..ae7a41b3045 100644 --- a/tools/ci-build/publisher/src/subcommand/claim_crate_names.rs +++ b/tools/ci-build/publisher/src/subcommand/claim_crate_names.rs @@ -3,12 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ use crate::fs::Fs; -use crate::package::{discover_package_manifests, Error, PackageHandle}; +use crate::package::{discover_packages, PackageHandle, Publish}; use crate::publish::{has_been_published_on_crates_io, publish}; use crate::subcommand::publish::correct_owner; use crate::{cargo, SDK_REPO_NAME}; -use anyhow::Context; -use cargo_toml::Manifest; use clap::Parser; use dialoguer::Confirm; use semver::Version; @@ -79,20 +77,11 @@ async fn discover_publishable_crate_names(repository_root: &Path) -> anyhow::Res fs: Fs, path: PathBuf, ) -> anyhow::Result> { - let manifest_paths = discover_package_manifests(path).await?; + let packages = discover_packages(fs, path).await?; let mut publishable_package_names = HashSet::new(); - for manifest_path in manifest_paths { - let contents: Vec = fs.read_file(&manifest_path).await?; - let manifest = Manifest::from_slice(&contents).with_context(|| { - format!("failed to load package manifest for {:?}", manifest_path) - })?; - let package = manifest - .package - .ok_or(Error::InvalidManifest(manifest_path)) - .context("crate manifest doesn't have a `[package]` section")?; - let name = package.name; - if let cargo_toml::Publish::Flag(true) = package.publish { - publishable_package_names.insert(name); + for package in packages { + if let Publish::Allowed = package.publish { + publishable_package_names.insert(package.handle.name); } } Ok(publishable_package_names) diff --git a/tools/ci-build/publisher/src/subcommand/fix_manifests.rs b/tools/ci-build/publisher/src/subcommand/fix_manifests.rs index c6c15afcebb..910f5fd51c6 100644 --- a/tools/ci-build/publisher/src/subcommand/fix_manifests.rs +++ b/tools/ci-build/publisher/src/subcommand/fix_manifests.rs @@ -10,7 +10,7 @@ //! version numbers in addition to the dependency path. use crate::fs::Fs; -use crate::package::{discover_package_manifests, parse_version}; +use crate::package::{discover_manifests, parse_version}; use crate::SDK_REPO_NAME; use anyhow::{bail, Context, Result}; use clap::Parser; @@ -20,6 +20,7 @@ use std::collections::BTreeMap; use std::ffi::OsStr; use std::path::{Path, PathBuf}; use toml::value::Table; +use toml::Value; use tracing::info; mod validate; @@ -55,7 +56,7 @@ pub async fn subcommand_fix_manifests( true => Mode::Check, false => Mode::Execute, }; - let manifest_paths = discover_package_manifests(location.into()).await?; + let manifest_paths = discover_manifests(location.into()).await?; let mut manifests = read_manifests(Fs::Real, manifest_paths).await?; let versions = package_versions(&manifests)?; @@ -91,6 +92,15 @@ fn package_versions(manifests: &[Manifest]) -> Result> Some(package) => package, None => continue, }; + // ignore non-publishable crates + if let Some(Value::Boolean(false)) = manifest + .metadata + .get("package") + .expect("checked above") + .get("publish") + { + continue; + } let name = package .get("name") .and_then(|name| name.as_str()) diff --git a/tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs b/tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs index ffa5eabca19..6b9ac35397b 100644 --- a/tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs +++ b/tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs @@ -4,7 +4,7 @@ */ use crate::fs::Fs; -use crate::package::{discover_package_manifests, read_packages}; +use crate::package::discover_packages; use anyhow::{bail, Context, Result}; use clap::Parser; use semver::Version; @@ -70,10 +70,7 @@ pub async fn subcommand_generate_version_manifest( (None, Some(output_location)) => output_location, _ => bail!("Only one of `--location` or `--output-location` should be provided"), }; - let manifests = discover_package_manifests(input_location.into()) - .await - .context("discover package manifests")?; - let packages = read_packages(Fs::Real, manifests) + let packages = discover_packages(Fs::Real, input_location.into()) .await .context("read packages")?; diff --git a/tools/ci-build/sdk-versioner/Cargo.lock b/tools/ci-build/sdk-versioner/Cargo.lock index 4da329eedd1..eafd9037eed 100644 --- a/tools/ci-build/sdk-versioner/Cargo.lock +++ b/tools/ci-build/sdk-versioner/Cargo.lock @@ -572,6 +572,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "pathdiff" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8835116a5c179084a830efb3adc117ab007512b535bc1a21c991d3b32a6b44dd" + [[package]] name = "percent-encoding" version = "2.2.0" @@ -748,6 +754,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", + "pathdiff", "pretty_assertions", "smithy-rs-tool-common", "tempfile", diff --git a/tools/ci-build/sdk-versioner/Cargo.toml b/tools/ci-build/sdk-versioner/Cargo.toml index 7a20bbdd14a..ee732cc4301 100644 --- a/tools/ci-build/sdk-versioner/Cargo.toml +++ b/tools/ci-build/sdk-versioner/Cargo.toml @@ -15,6 +15,7 @@ opt-level = 0 [dependencies] anyhow = "1.0" clap = { version = "~3.1.18", features = ["derive"] } +pathdiff = "0.2.1" smithy-rs-tool-common = { version = "0.1", path = "../smithy-rs-tool-common" } toml_edit = { version = "0.19.6" } diff --git a/tools/ci-build/sdk-versioner/src/main.rs b/tools/ci-build/sdk-versioner/src/main.rs index 77cf9ff6061..63ffd6bd2f3 100644 --- a/tools/ci-build/sdk-versioner/src/main.rs +++ b/tools/ci-build/sdk-versioner/src/main.rs @@ -29,6 +29,7 @@ enum Args { /// Path(s) to recursively update Cargo.toml files in #[clap()] crate_paths: Vec, + // TODO(https://github.com/awslabs/smithy-rs/issues/2810): `isolate_crates` can be removed once the `Flat` example directory structure is cleaned up /// Makes each individual crate its own workspace #[clap(long)] isolate_crates: bool, @@ -41,6 +42,7 @@ enum Args { /// Path(s) to recursively update Cargo.toml files in #[clap()] crate_paths: Vec, + // TODO(https://github.com/awslabs/smithy-rs/issues/2810): `isolate_crates` can be removed once the `Flat` example directory structure is cleaned up /// Makes each individual crate its own workspace #[clap(long)] isolate_crates: bool, @@ -56,6 +58,7 @@ enum Args { /// Path(s) to recursively update Cargo.toml files in #[clap()] crate_paths: Vec, + // TODO(https://github.com/awslabs/smithy-rs/issues/2810): `isolate_crates` can be removed once the `Flat` example directory structure is cleaned up /// Makes each individual crate its own workspace #[clap(long)] isolate_crates: bool, @@ -87,8 +90,26 @@ impl Args { } } -struct DependencyContext<'a> { - sdk_path: Option<&'a Path>, +// TODO(https://github.com/awslabs/smithy-rs/issues/2810): Remove `SdkPath` and just use a `PathBuf` with the new logic +// This is only around for backwards compatibility for the next release's sync process0 +enum SdkPath { + /// Don't even attempt to resolve the correct relative path to dependencies + UseDumbLogic(PathBuf), + /// Resolve the correct relative path to dependencies + UseNewLogic(PathBuf), +} +impl From<&PathBuf> for SdkPath { + fn from(value: &PathBuf) -> Self { + if !value.exists() { + SdkPath::UseDumbLogic(value.into()) + } else { + SdkPath::UseNewLogic(value.into()) + } + } +} + +struct DependencyContext { + sdk_path: Option, versions_manifest: Option, } @@ -96,7 +117,7 @@ fn main() -> Result<()> { let args = Args::parse().validate()?; let dependency_context = match &args { Args::UsePathDependencies { sdk_path, .. } => DependencyContext { - sdk_path: Some(sdk_path), + sdk_path: Some(sdk_path.into()), versions_manifest: None, }, Args::UseVersionDependencies { versions_toml, .. } => DependencyContext { @@ -108,7 +129,7 @@ fn main() -> Result<()> { versions_toml, .. } => DependencyContext { - sdk_path: Some(sdk_path), + sdk_path: Some(sdk_path.into()), versions_manifest: Some(VersionsManifest::from_file(versions_toml)?), }, }; @@ -133,6 +154,7 @@ fn update_manifest( isolate_crates: bool, ) -> anyhow::Result<()> { println!("Updating {:?}...", manifest_path); + let crate_path = manifest_path.parent().expect("manifest has a parent"); let mut metadata: Document = String::from_utf8( fs::read(manifest_path).with_context(|| format!("failed to read {manifest_path:?}"))?, @@ -150,9 +172,11 @@ fn update_manifest( manifest_path ); } - changed = - update_dependencies(dependencies.as_table_mut().unwrap(), dependency_context)? - || changed; + changed = update_dependencies( + dependencies.as_table_mut().unwrap(), + dependency_context, + crate_path, + )? || changed; } } if isolate_crates && !metadata.contains_key("workspace") { @@ -177,6 +201,7 @@ fn update_manifest( fn update_dependencies( dependencies: &mut Table, dependency_context: &DependencyContext, + crate_path: &Path, ) -> Result { let mut changed = false; for (key, value) in dependencies.iter_mut() { @@ -191,6 +216,7 @@ fn update_dependencies( key.get(), old_value, dependency_context, + crate_path, )?)); changed = true; } @@ -210,10 +236,14 @@ fn crate_path_name(name: &str) -> &str { } fn updated_dependency_value( - crate_name: &str, + dependency_name: &str, old_value: Table, dependency_context: &DependencyContext, + crate_path: &Path, ) -> Result { + let crate_path = crate_path + .canonicalize() + .context("failed to canonicalize crate path")?; let mut value = old_value; // Remove keys that will be replaced @@ -223,25 +253,45 @@ fn updated_dependency_value( value.remove("path"); // Set the `path` if one was given - if let Some(path) = &dependency_context.sdk_path { - let crate_path = path.join(crate_path_name(crate_name)); - value["path"] = toml_edit::value( - crate_path - .as_os_str() - .to_str() - .expect("valid utf-8 path") - .to_string(), - ); + match &dependency_context.sdk_path { + Some(SdkPath::UseDumbLogic(sdk_path)) => { + let crate_path = sdk_path.join(crate_path_name(dependency_name)); + value["path"] = toml_edit::value( + crate_path + .as_os_str() + .to_str() + .expect("valid utf-8 path") + .to_string(), + ); + } + Some(SdkPath::UseNewLogic(sdk_path)) => { + let dependency_path = sdk_path + .join(crate_path_name(dependency_name)) + .canonicalize() + .context("failed to canonicalize dependency path")?; + if let Some(relative_path) = pathdiff::diff_paths(&dependency_path, &crate_path) { + value["path"] = toml_edit::value( + relative_path + .as_os_str() + .to_str() + .expect("valid utf-8 path") + .to_string(), + ); + } else { + bail!("Failed to create relative path from {crate_path:?} to {dependency_path:?}"); + } + } + _ => {} } // Set the `version` if one was given if let Some(manifest) = &dependency_context.versions_manifest { - if let Some(crate_metadata) = manifest.crates.get(crate_name) { + if let Some(crate_metadata) = manifest.crates.get(dependency_name) { value["version"] = toml_edit::value(crate_metadata.version.clone()); } else { bail!( "Crate `{}` was missing from the `versions.toml`", - crate_name + dependency_name ); } } @@ -270,11 +320,12 @@ fn discover_manifests(manifests: &mut Vec, path: impl AsRef) -> a #[cfg(test)] mod tests { - use crate::{update_manifest, DependencyContext}; + use crate::{crate_path_name, update_manifest, DependencyContext, SdkPath}; use pretty_assertions::assert_eq; use smithy_rs_tool_common::package::PackageCategory; use smithy_rs_tool_common::versions_manifest::{CrateVersion, VersionsManifest}; use std::path::PathBuf; + use std::{fs, process}; fn versions_toml_for(crates: &[(&str, &str)]) -> VersionsManifest { VersionsManifest { @@ -321,12 +372,47 @@ features = ["foo", "baz"] "#; #[track_caller] - fn test_with_context(isolate_crates: bool, context: DependencyContext, expected: &[u8]) { - let manifest_file = tempfile::NamedTempFile::new().unwrap(); - let manifest_path = manifest_file.into_temp_path(); + fn test_with_context( + isolate_crates: bool, + crate_path_rel: &str, + sdk_crates: &[&'static str], + context: DependencyContext, + expected: &[u8], + ) { + let temp_dir = tempfile::tempdir().unwrap(); + let crate_path = temp_dir.path().join(crate_path_rel); + fs::create_dir_all(&crate_path).unwrap(); + + let manifest_path = crate_path.join("Cargo.toml"); std::fs::write(&manifest_path, TEST_MANIFEST).unwrap(); - update_manifest(&manifest_path, &context, isolate_crates).expect("success"); + if let Some(SdkPath::UseNewLogic(sdk_path)) = context.sdk_path.as_ref() { + for sdk_crate in sdk_crates { + let sdk_crate_path = temp_dir + .path() + .join(sdk_path) + .join(crate_path_name(sdk_crate)); + fs::create_dir_all(sdk_crate_path).unwrap(); + } + } + // Assist with debugging when the tests fail + if let Ok(output) = process::Command::new("find").arg(temp_dir.path()).output() { + println!( + "Test directory structure:\n{}", + String::from_utf8_lossy(&output.stdout) + ); + } + + let fixed_context = if let Some(SdkPath::UseNewLogic(sdk_path)) = context.sdk_path.as_ref() + { + DependencyContext { + sdk_path: Some(SdkPath::UseNewLogic(temp_dir.path().join(sdk_path))), + versions_manifest: context.versions_manifest, + } + } else { + context + }; + update_manifest(&manifest_path, &fixed_context, isolate_crates).expect("success"); let actual = String::from_utf8(std::fs::read(&manifest_path).expect("read tmp file")).unwrap(); @@ -338,6 +424,8 @@ features = ["foo", "baz"] fn update_dependencies_with_versions() { test_with_context( false, + "examples/foo", + &[], DependencyContext { sdk_path: None, versions_manifest: Some(versions_toml_for(&[ @@ -374,8 +462,54 @@ features = ["foo", "baz"] fn update_dependencies_with_paths() { test_with_context( false, + "path/to/test", + &[ + "aws-config", + "aws-sdk-s3", + "aws-smithy-types", + "aws-smithy-http", + ], + DependencyContext { + sdk_path: Some(SdkPath::UseNewLogic(PathBuf::from("sdk"))), + versions_manifest: None, + }, + br#" +[package] +name = "test" +version = "0.1.0" + +# Some comment that should be preserved +[dependencies] +aws-config = { path = "../../../sdk/aws-config" } +aws-sdk-s3 = { path = "../../../sdk/s3" } +aws-smithy-types = { path = "../../../sdk/aws-smithy-types" } +aws-smithy-http = { path = "../../../sdk/aws-smithy-http", features = ["test-util"] } +something-else = { version = "0.1", no-default-features = true } +tokio = { version = "1.18", features = ["net"] } + +[dev-dependencies.another-thing] +# some comment +version = "5.0" +# another comment +features = ["foo", "baz"] +"#, + ); + } + + // TODO(https://github.com/awslabs/smithy-rs/issues/2810): Remove this test + #[test] + fn update_dependencies_with_paths_dumb_logic() { + test_with_context( + false, + "path/to/test", + &[ + "aws-config", + "aws-sdk-s3", + "aws-smithy-types", + "aws-smithy-http", + ], DependencyContext { - sdk_path: Some(&PathBuf::from("/foo/asdf/")), + sdk_path: Some(SdkPath::UseDumbLogic(PathBuf::from("a/dumb/path/to"))), versions_manifest: None, }, br#" @@ -385,10 +519,10 @@ version = "0.1.0" # Some comment that should be preserved [dependencies] -aws-config = { path = "/foo/asdf/aws-config" } -aws-sdk-s3 = { path = "/foo/asdf/s3" } -aws-smithy-types = { path = "/foo/asdf/aws-smithy-types" } -aws-smithy-http = { path = "/foo/asdf/aws-smithy-http", features = ["test-util"] } +aws-config = { path = "a/dumb/path/to/aws-config" } +aws-sdk-s3 = { path = "a/dumb/path/to/s3" } +aws-smithy-types = { path = "a/dumb/path/to/aws-smithy-types" } +aws-smithy-http = { path = "a/dumb/path/to/aws-smithy-http", features = ["test-util"] } something-else = { version = "0.1", no-default-features = true } tokio = { version = "1.18", features = ["net"] } @@ -405,8 +539,15 @@ features = ["foo", "baz"] fn update_dependencies_with_versions_and_paths() { test_with_context( false, + "deep/path/to/test", + &[ + "aws-config", + "aws-sdk-s3", + "aws-smithy-types", + "aws-smithy-http", + ], DependencyContext { - sdk_path: Some(&PathBuf::from("/foo/asdf/")), + sdk_path: Some(SdkPath::UseNewLogic(PathBuf::from("sdk"))), versions_manifest: Some(versions_toml_for(&[ ("aws-config", "0.5.0"), ("aws-sdk-s3", "0.13.0"), @@ -421,10 +562,10 @@ version = "0.1.0" # Some comment that should be preserved [dependencies] -aws-config = { version = "0.5.0", path = "/foo/asdf/aws-config" } -aws-sdk-s3 = { version = "0.13.0", path = "/foo/asdf/s3" } -aws-smithy-types = { version = "0.10.0", path = "/foo/asdf/aws-smithy-types" } -aws-smithy-http = { version = "0.9.0", path = "/foo/asdf/aws-smithy-http", features = ["test-util"] } +aws-config = { version = "0.5.0", path = "../../../../sdk/aws-config" } +aws-sdk-s3 = { version = "0.13.0", path = "../../../../sdk/s3" } +aws-smithy-types = { version = "0.10.0", path = "../../../../sdk/aws-smithy-types" } +aws-smithy-http = { version = "0.9.0", path = "../../../../sdk/aws-smithy-http", features = ["test-util"] } something-else = { version = "0.1", no-default-features = true } tokio = { version = "1.18", features = ["net"] } @@ -441,8 +582,15 @@ features = ["foo", "baz"] fn update_dependencies_isolate_crates() { test_with_context( true, + "deep/path/to/test", + &[ + "aws-config", + "aws-sdk-s3", + "aws-smithy-types", + "aws-smithy-http", + ], DependencyContext { - sdk_path: Some(&PathBuf::from("/foo/asdf/")), + sdk_path: Some(SdkPath::UseNewLogic(PathBuf::from("sdk"))), versions_manifest: Some(versions_toml_for(&[ ("aws-config", "0.5.0"), ("aws-sdk-s3", "0.13.0"), @@ -459,10 +607,10 @@ version = "0.1.0" # Some comment that should be preserved [dependencies] -aws-config = { version = "0.5.0", path = "/foo/asdf/aws-config" } -aws-sdk-s3 = { version = "0.13.0", path = "/foo/asdf/s3" } -aws-smithy-types = { version = "0.10.0", path = "/foo/asdf/aws-smithy-types" } -aws-smithy-http = { version = "0.9.0", path = "/foo/asdf/aws-smithy-http", features = ["test-util"] } +aws-config = { version = "0.5.0", path = "../../../../sdk/aws-config" } +aws-sdk-s3 = { version = "0.13.0", path = "../../../../sdk/s3" } +aws-smithy-types = { version = "0.10.0", path = "../../../../sdk/aws-smithy-types" } +aws-smithy-http = { version = "0.9.0", path = "../../../../sdk/aws-smithy-http", features = ["test-util"] } something-else = { version = "0.1", no-default-features = true } tokio = { version = "1.18", features = ["net"] } diff --git a/tools/ci-scripts/check-aws-sdk-examples b/tools/ci-scripts/check-aws-sdk-examples index 46495665a01..db5880b499e 100755 --- a/tools/ci-scripts/check-aws-sdk-examples +++ b/tools/ci-scripts/check-aws-sdk-examples @@ -13,6 +13,6 @@ cd aws-sdk/examples for example in *; do echo -e "${C_YELLOW}Checking examples/${example}...${C_RESET}" pushd "${example}" &>/dev/null - cargo check + cargo check && cargo clean popd &>/dev/null done diff --git a/tools/ci-scripts/generate-aws-sdk b/tools/ci-scripts/generate-aws-sdk index 818e5361268..18b29529ad9 100755 --- a/tools/ci-scripts/generate-aws-sdk +++ b/tools/ci-scripts/generate-aws-sdk @@ -25,7 +25,8 @@ echo -e "${C_YELLOW}Taking examples from 'awsdocs/aws-doc-sdk-examples'...${C_RE examples_revision=$(cd aws-doc-sdk-examples; git rev-parse HEAD) mv aws-doc-sdk-examples/rust_dev_preview smithy-rs/aws/sdk/examples rm -rf smithy-rs/aws/sdk/examples/.cargo -rm smithy-rs/aws/sdk/examples/Cargo.toml +# TODO(https://github.com/awslabs/smithy-rs/issues/2810): This Cargo.toml `rm` can be removed when the flat example structure is cleaned up +rm -f smithy-rs/aws/sdk/examples/Cargo.toml echo -e "${C_YELLOW}Creating empty model metadata file since we don't have model update information...${C_RESET}" MODEL_METADATA_PATH="$(pwd)/model-metadata.toml" From 71b401f5bf22904323d2ddeedc71d423c2d3d822 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 30 Jun 2023 10:48:41 -0700 Subject: [PATCH 2/6] Move endpoint module and add endpoint re-exports (#2822) All the types in the `crate::endpoint` module are related to endpoint configuration, so they should be moved into `crate::config::endpoint` to be consistent with the rest of the generated crate organization. This PR moves them for the orchestrator implementation, but retains them in `crate::endpoint` when generating for middleware. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../codegen/client/smithy/ClientRustModule.kt | 16 +++++++++++++++- .../endpoint/EndpointConfigCustomization.kt | 8 +++++--- .../smithy/endpoint/EndpointTypesGenerator.kt | 6 +++--- .../smithy/endpoint/EndpointsDecorator.kt | 6 +++--- .../generators/EndpointParamsGenerator.kt | 19 +++++++++++-------- .../generators/EndpointResolverGenerator.kt | 16 ++++++++++------ .../ClientRuntimeTypesReExportGenerator.kt | 10 ++++++++++ .../endpoint/EndpointParamsGeneratorTest.kt | 4 +++- .../endpoint/EndpointResolverGeneratorTest.kt | 13 ++++++++----- 9 files changed, 68 insertions(+), 30 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt index 5f46cdafb10..93a0c2de162 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt @@ -60,6 +60,9 @@ object ClientRustModule { /** crate::client */ val self = RustModule.public("config") + /** crate::config::endpoint */ + val endpoint = RustModule.public("endpoint", parent = self) + /** crate::config::retry */ val retry = RustModule.public("retry", parent = self) @@ -70,8 +73,18 @@ object ClientRustModule { val interceptors = RustModule.public("interceptors", parent = self) } - val Error = RustModule.public("error") + // TODO(enableNewSmithyRuntimeCleanup): Delete this root endpoint module + @Deprecated(message = "use the endpoint() method to get the endpoint module for now") val Endpoint = RustModule.public("endpoint") + + // TODO(enableNewSmithyRuntimeCleanup): Just use Config.endpoint directly and delete this function + fun endpoint(codegenContext: ClientCodegenContext): RustModule.LeafModule = if (codegenContext.smithyRuntimeMode.defaultToMiddleware) { + Endpoint + } else { + Config.endpoint + } + + val Error = RustModule.public("error") val Operation = RustModule.public("operation") val Meta = RustModule.public("meta") val Input = RustModule.public("input") @@ -99,6 +112,7 @@ class ClientModuleDocProvider( ClientRustModule.client -> clientModuleDoc() ClientRustModule.Client.customize -> customizeModuleDoc() ClientRustModule.config -> strDoc("Configuration for $serviceName.") + ClientRustModule.Config.endpoint -> strDoc("Types needed to configure endpoint resolution.") ClientRustModule.Config.retry -> strDoc("Retry configuration.") ClientRustModule.Config.timeout -> strDoc("Timeout configuration.") ClientRustModule.Config.interceptors -> strDoc("Types needed to implement [`Interceptor`](crate::config::Interceptor).") diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt index 9cfc92e92d9..a6b07ecaaad 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt @@ -19,7 +19,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.pre * Customization which injects an Endpoints 2.0 Endpoint Resolver into the service config struct */ internal class EndpointConfigCustomization( - codegenContext: ClientCodegenContext, + private val codegenContext: ClientCodegenContext, private val typesGenerator: EndpointTypesGenerator, ) : ConfigCustomization() { @@ -86,6 +86,8 @@ internal class EndpointConfigCustomization( ServiceConfig.BuilderImpl -> { // if there are no rules, we don't generate a default resolver—we need to also suppress those docs. val defaultResolverDocs = if (typesGenerator.defaultResolver() != null) { + val endpointModule = ClientRustModule.endpoint(codegenContext).fullyQualifiedPath() + .replace("crate::", "$moduleUseName::") """ /// /// When unset, the client will used a generated endpoint resolver based on the endpoint resolution @@ -94,7 +96,7 @@ internal class EndpointConfigCustomization( /// ## Examples /// ```no_run /// use aws_smithy_http::endpoint; - /// use $moduleUseName::endpoint::{Params as EndpointParams, DefaultResolver}; + /// use $endpointModule::{Params as EndpointParams, DefaultResolver}; /// /// Endpoint resolver which adds a prefix to the generated endpoint /// ##[derive(Debug)] /// struct PrefixResolver { @@ -193,7 +195,7 @@ internal class EndpointConfigCustomization( } } else { val alwaysFailsResolver = - RuntimeType.forInlineFun("MissingResolver", ClientRustModule.Endpoint) { + RuntimeType.forInlineFun("MissingResolver", ClientRustModule.endpoint(codegenContext)) { rustTemplate( """ ##[derive(Debug)] diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointTypesGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointTypesGenerator.kt index 0906a4f6db1..889b741ebbc 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointTypesGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointTypesGenerator.kt @@ -40,10 +40,10 @@ class EndpointTypesGenerator( } } - fun paramsStruct(): RuntimeType = EndpointParamsGenerator(params).paramsStruct() - fun paramsBuilder(): RuntimeType = EndpointParamsGenerator(params).paramsBuilder() + fun paramsStruct(): RuntimeType = EndpointParamsGenerator(codegenContext, params).paramsStruct() + fun paramsBuilder(): RuntimeType = EndpointParamsGenerator(codegenContext, params).paramsBuilder() fun defaultResolver(): RuntimeType? = - rules?.let { EndpointResolverGenerator(stdlib, runtimeConfig).defaultEndpointResolver(it) } + rules?.let { EndpointResolverGenerator(codegenContext, stdlib).defaultEndpointResolver(it) } fun testGenerator(): Writable = defaultResolver()?.let { diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt index e9176ff7672..351d205813c 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt @@ -18,7 +18,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.CustomRuntimeFunction import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointParamsGenerator -import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointTests +import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.endpointTestsModule import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rulesgen.SmithyEndpointsStdLib import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationSection @@ -134,8 +134,8 @@ class EndpointsDecorator : ClientCodegenDecorator { override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) { val generator = EndpointTypesGenerator.fromContext(codegenContext) - rustCrate.withModule(ClientRustModule.Endpoint) { - withInlineModule(EndpointTests, rustCrate.moduleDocProvider) { + rustCrate.withModule(ClientRustModule.endpoint(codegenContext)) { + withInlineModule(endpointTestsModule(codegenContext), rustCrate.moduleDocProvider) { generator.testGenerator()(this) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt index 73752e12c95..779a4eb8d27 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators import software.amazon.smithy.rulesengine.language.eval.Value import software.amazon.smithy.rulesengine.language.syntax.Identifier import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameters +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.client.smithy.endpoint.memberName import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName @@ -37,12 +38,12 @@ import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.orNull // internals contains the actual resolver function -val EndpointImpl = RustModule.private("internals", parent = ClientRustModule.Endpoint) +fun endpointImplModule(codegenContext: ClientCodegenContext) = RustModule.private("internals", parent = ClientRustModule.endpoint(codegenContext)) -val EndpointTests = RustModule.new( +fun endpointTestsModule(codegenContext: ClientCodegenContext) = RustModule.new( "test", visibility = Visibility.PRIVATE, - parent = ClientRustModule.Endpoint, + parent = ClientRustModule.endpoint(codegenContext), inline = true, documentationOverride = "", ).cfgTest() @@ -108,22 +109,24 @@ val EndpointStdLib = RustModule.private("endpoint_lib") * ``` */ -internal class EndpointParamsGenerator(private val parameters: Parameters) { - +internal class EndpointParamsGenerator( + private val codegenContext: ClientCodegenContext, + private val parameters: Parameters, +) { companion object { fun memberName(parameterName: String) = Identifier.of(parameterName).rustName() fun setterName(parameterName: String) = "set_${memberName(parameterName)}" } - fun paramsStruct(): RuntimeType = RuntimeType.forInlineFun("Params", ClientRustModule.Endpoint) { + fun paramsStruct(): RuntimeType = RuntimeType.forInlineFun("Params", ClientRustModule.endpoint(codegenContext)) { generateEndpointsStruct(this) } - internal fun paramsBuilder(): RuntimeType = RuntimeType.forInlineFun("ParamsBuilder", ClientRustModule.Endpoint) { + internal fun paramsBuilder(): RuntimeType = RuntimeType.forInlineFun("ParamsBuilder", ClientRustModule.endpoint(codegenContext)) { generateEndpointParamsBuilder(this) } - private fun paramsError(): RuntimeType = RuntimeType.forInlineFun("InvalidParams", ClientRustModule.Endpoint) { + private fun paramsError(): RuntimeType = RuntimeType.forInlineFun("InvalidParams", ClientRustModule.endpoint(codegenContext)) { rust( """ /// An error that occurred during endpoint resolution diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointResolverGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointResolverGenerator.kt index 7dbede95968..a8237205711 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointResolverGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointResolverGenerator.kt @@ -14,6 +14,7 @@ import software.amazon.smithy.rulesengine.language.syntax.fn.IsSet import software.amazon.smithy.rulesengine.language.syntax.rule.Condition import software.amazon.smithy.rulesengine.language.syntax.rule.Rule import software.amazon.smithy.rulesengine.language.visit.RuleValueVisitor +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.client.smithy.endpoint.Context import software.amazon.smithy.rust.codegen.client.smithy.endpoint.Types @@ -33,7 +34,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.toType import software.amazon.smithy.rust.codegen.core.rustlang.writable -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.orNull @@ -119,7 +119,11 @@ class FunctionRegistry(private val functions: List) { * */ -internal class EndpointResolverGenerator(stdlib: List, runtimeConfig: RuntimeConfig) { +internal class EndpointResolverGenerator( + private val codegenContext: ClientCodegenContext, + stdlib: List, +) { + private val runtimeConfig = codegenContext.runtimeConfig private val registry: FunctionRegistry = FunctionRegistry(stdlib) private val types = Types(runtimeConfig) private val codegenScope = arrayOf( @@ -164,7 +168,7 @@ internal class EndpointResolverGenerator(stdlib: List, ru // Now that we rendered the rules once (and then threw it away) we can see what functions we actually used! val fnsUsed = registry.fnsUsed() - return RuntimeType.forInlineFun("DefaultResolver", ClientRustModule.Endpoint) { + return RuntimeType.forInlineFun("DefaultResolver", ClientRustModule.endpoint(codegenContext)) { rustTemplate( """ /// The default endpoint resolver @@ -190,7 +194,7 @@ internal class EndpointResolverGenerator(stdlib: List, ru """, "custom_fields" to fnsUsed.mapNotNull { it.structField() }.join(","), "custom_fields_init" to fnsUsed.mapNotNull { it.structFieldInit() }.join(","), - "Params" to EndpointParamsGenerator(endpointRuleSet.parameters).paramsStruct(), + "Params" to EndpointParamsGenerator(codegenContext, endpointRuleSet.parameters).paramsStruct(), "additional_args" to fnsUsed.mapNotNull { it.additionalArgsInvocation("self") }.join(","), "resolver_fn" to resolverFn(endpointRuleSet, fnsUsed), *codegenScope, @@ -202,7 +206,7 @@ internal class EndpointResolverGenerator(stdlib: List, ru endpointRuleSet: EndpointRuleSet, fnsUsed: List, ): RuntimeType { - return RuntimeType.forInlineFun("resolve_endpoint", EndpointImpl) { + return RuntimeType.forInlineFun("resolve_endpoint", endpointImplModule(codegenContext)) { Attribute(allow(allowLintsForResolver)).render(this) rustTemplate( """ @@ -212,7 +216,7 @@ internal class EndpointResolverGenerator(stdlib: List, ru """, *codegenScope, - "Params" to EndpointParamsGenerator(endpointRuleSet.parameters).paramsStruct(), + "Params" to EndpointParamsGenerator(codegenContext, endpointRuleSet.parameters).paramsStruct(), "additional_args" to fnsUsed.mapNotNull { it.additionalArgsSignature() }.join(","), "body" to resolverFnBody(endpointRuleSet), ) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt index 71d8ab9e4cc..408ff44390f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientRuntimeTypesReExportGenerator.kt @@ -33,6 +33,16 @@ class ClientRuntimeTypesReExportGenerator( "Interceptor" to RuntimeType.interceptor(rc), ) } + rustCrate.withModule(ClientRustModule.endpoint(codegenContext)) { + rustTemplate( + """ + pub use #{ResolveEndpoint}; + pub use #{SharedEndpointResolver}; + """, + "ResolveEndpoint" to RuntimeType.smithyHttp(rc).resolve("endpoint::ResolveEndpoint"), + "SharedEndpointResolver" to RuntimeType.smithyHttp(rc).resolve("endpoint::SharedEndpointResolver"), + ) + } rustCrate.withModule(ClientRustModule.Config.retry) { rustTemplate( """ diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsGeneratorTest.kt index 6397e117e14..34636f596c5 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointParamsGeneratorTest.kt @@ -9,6 +9,7 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import software.amazon.smithy.rulesengine.testutil.TestDiscovery import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointParamsGenerator +import software.amazon.smithy.rust.codegen.client.testutil.testClientCodegenContext import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest @@ -25,6 +26,7 @@ internal class EndpointParamsGeneratorTest { @MethodSource("testSuites") fun `generate endpoint params for provided test suites`(testSuite: TestDiscovery.RulesTestSuite) { val project = TestWorkspace.testProject() + val context = testClientCodegenContext() project.lib { unitTest("params_work") { rustTemplate( @@ -32,7 +34,7 @@ internal class EndpointParamsGeneratorTest { // this might fail if there are required fields let _ = #{Params}::builder().build(); """, - "Params" to EndpointParamsGenerator(testSuite.ruleSet().parameters).paramsStruct(), + "Params" to EndpointParamsGenerator(context, testSuite.ruleSet().parameters).paramsStruct(), ) } } diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointResolverGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointResolverGeneratorTest.kt index b842705f994..5a798cae4d5 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointResolverGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointResolverGeneratorTest.kt @@ -53,15 +53,16 @@ class EndpointResolverGeneratorTest { // return } val project = TestWorkspace.testProject() + val context = testClientCodegenContext() suite.ruleSet().typecheck() project.lib { val ruleset = EndpointResolverGenerator( + context, SmithyEndpointsStdLib + awsStandardLib(TestRuntimeConfig, partitionsJson), - TestRuntimeConfig, ).defaultEndpointResolver(suite.ruleSet()) val testGenerator = EndpointTestGenerator( suite.testSuite().testCases, - paramsType = EndpointParamsGenerator(suite.ruleSet().parameters).paramsStruct(), + paramsType = EndpointParamsGenerator(context, suite.ruleSet().parameters).paramsStruct(), resolverType = ruleset, suite.ruleSet().parameters, codegenContext = testClientCodegenContext(model = Model.builder().build()), @@ -79,15 +80,16 @@ class EndpointResolverGeneratorTest { testSuites().filter { it.ruleSet().sourceLocation.filename.endsWith("/uri-encode.json") }.findFirst() .orElseThrow() val project = TestWorkspace.testProject() + val context = testClientCodegenContext() suite.ruleSet().typecheck() project.lib { val ruleset = EndpointResolverGenerator( + context, SmithyEndpointsStdLib + awsStandardLib(TestRuntimeConfig, partitionsJson), - TestRuntimeConfig, ).defaultEndpointResolver(suite.ruleSet()) val testGenerator = EndpointTestGenerator( suite.testSuite().testCases, - paramsType = EndpointParamsGenerator(suite.ruleSet().parameters).paramsStruct(), + paramsType = EndpointParamsGenerator(context, suite.ruleSet().parameters).paramsStruct(), resolverType = ruleset, suite.ruleSet().parameters, codegenContext = testClientCodegenContext(Model.builder().build()), @@ -115,7 +117,8 @@ class EndpointResolverGeneratorTest { val scope = Scope() scope.insert("Region", Type.string()) endpoint.typeCheck(scope) - val generator = EndpointResolverGenerator(listOf(), TestRuntimeConfig) + val context = testClientCodegenContext() + val generator = EndpointResolverGenerator(context, listOf()) TestWorkspace.testProject().unitTest { rustTemplate( """ From 386ded887d108940080b3d096ed29aa66b43f39a Mon Sep 17 00:00:00 2001 From: Zelda Hessler Date: Fri, 30 Jun 2023 14:09:51 -0500 Subject: [PATCH 3/6] add connection poisoning support to the orchestrator (#2824) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR updates the orchestrator to support connection poisoning for the `hyper` connector. It also renames many usages of "connection" to "connector" in order to make talking about these concepts less confusing. In short: ``` /// A "connector" manages one or more "connections", handles connection timeouts, re-establishes /// connections, etc. /// /// "Connections" refers to the actual transport layer implementation of the connector. /// By default, the orchestrator uses a connector provided by `hyper`. ``` One possibly controversial decision I made is that `reconnect_mode` is now a standalone configurable field, rather that a sub-field of `RetryConfig`. I think we should get together as a team and discuss what kinds of things we want to allow users to do when configuring connections. My thoughts on this are that we should either: - **A.** Make connection-related settings their own types instead of including them within other config types - **B.** Make a single config struct for all connector-related stuff/use the preëxisting `ConnectorSettings` struct. Personally, I'm partial to A. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti --- .../rustsdk/RetryClassifierDecorator.kt | 11 +- .../timestream/TimestreamDecorator.kt | 1 - aws/sdk/integration-tests/s3/Cargo.toml | 8 +- .../integration-tests/s3/tests/reconnects.rs | 30 ++-- .../ConnectionPoisoningConfigCustomization.kt | 37 +++++ .../HttpConnectorConfigDecorator.kt | 12 +- .../ResiliencyConfigCustomization.kt | 58 ++++---- .../customize/RequiredCustomizations.kt | 7 +- .../ServiceRuntimePluginGenerator.kt | 3 +- .../ResiliencyConfigCustomizationTest.kt | 2 +- .../rust/codegen/core/rustlang/RustType.kt | 1 + .../aws-smithy-runtime-api/src/client.rs | 3 + .../src/client/config_bag_accessors.rs | 15 +- .../src/client/connectors.rs | 31 ++++ .../client/interceptors/context/wrappers.rs | 10 ++ .../src/client/orchestrator.rs | 24 ---- rust-runtime/aws-smithy-runtime/src/client.rs | 10 +- .../client/{connections.rs => connectors.rs} | 10 +- .../client/connectors/connection_poisoning.rs | 132 ++++++++++++++++++ .../test_util.rs} | 9 +- .../src/client/orchestrator.rs | 25 +++- .../src/client/test_util.rs | 1 - .../src/client/test_util/connector.rs | 27 ---- rust-runtime/aws-smithy-types/src/retry.rs | 4 + 24 files changed, 336 insertions(+), 135 deletions(-) create mode 100644 codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt create mode 100644 rust-runtime/aws-smithy-runtime-api/src/client/connectors.rs rename rust-runtime/aws-smithy-runtime/src/client/{connections.rs => connectors.rs} (77%) create mode 100644 rust-runtime/aws-smithy-runtime/src/client/connectors/connection_poisoning.rs rename rust-runtime/aws-smithy-runtime/src/client/{connections/test_connection.rs => connectors/test_util.rs} (96%) delete mode 100644 rust-runtime/aws-smithy-runtime/src/client/test_util/connector.rs diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt index b9f901131dd..16f0644496d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryClassifierDecorator.kt @@ -15,6 +15,7 @@ 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.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.util.letIf class RetryClassifierDecorator : ClientCodegenDecorator { override val name: String = "RetryPolicy" @@ -25,10 +26,12 @@ class RetryClassifierDecorator : ClientCodegenDecorator { operation: OperationShape, baseCustomizations: List, ): List = - baseCustomizations + RetryClassifierFeature(codegenContext.runtimeConfig) + OperationRetryClassifiersFeature( - codegenContext, - operation, - ) + (baseCustomizations + RetryClassifierFeature(codegenContext.runtimeConfig)).letIf(codegenContext.smithyRuntimeMode.generateOrchestrator) { + it + OperationRetryClassifiersFeature( + codegenContext, + operation, + ) + } } class RetryClassifierFeature(private val runtimeConfig: RuntimeConfig) : OperationCustomization() { diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt index c066562ac36..57893b1c0ae 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt @@ -52,7 +52,6 @@ class TimestreamDecorator : ClientCodegenDecorator { Visibility.PUBLIC, CargoDependency.Tokio.copy(scope = DependencyScope.Compile, features = setOf("sync")), ) - val runtimeMode = codegenContext.smithyRuntimeMode rustCrate.lib { // helper function to resolve an endpoint given a base client rustTemplate( diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 554ff97101e..1dfcc942576 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -12,8 +12,8 @@ publish = false [dev-dependencies] async-std = "1.12.0" -aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] } aws-config = { path = "../../build/aws-sdk/sdk/aws-config" } +aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] } aws-http = { path = "../../build/aws-sdk/sdk/aws-http" } aws-sdk-s3 = { path = "../../build/aws-sdk/sdk/s3" } aws-sdk-sts = { path = "../../build/aws-sdk/sdk/sts" } @@ -36,9 +36,9 @@ serde_json = "1" smol = "1.2" tempfile = "3" tokio = { version = "1.23.1", features = ["macros", "test-util", "rt-multi-thread"] } -# If you're writing a test with this, take heed! `no-env-filter` means you'll be capturing -# logs from everything that speaks, so be specific with your asserts. -tracing-test = { version = "0.2.4", features = ["no-env-filter"] } tracing = "0.1.37" tracing-appender = "0.2.2" tracing-subscriber = { version = "0.3.15", features = ["env-filter", "json"] } +# If you're writing a test with this, take heed! `no-env-filter` means you'll be capturing +# logs from everything that speaks, so be specific with your asserts. +tracing-test = { version = "0.2.4", features = ["no-env-filter"] } diff --git a/aws/sdk/integration-tests/s3/tests/reconnects.rs b/aws/sdk/integration-tests/s3/tests/reconnects.rs index 91935319fbf..cb29bb2a66b 100644 --- a/aws/sdk/integration-tests/s3/tests/reconnects.rs +++ b/aws/sdk/integration-tests/s3/tests/reconnects.rs @@ -3,20 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_credential_types::provider::SharedCredentialsProvider; -use aws_credential_types::Credentials; -use aws_smithy_async::rt::sleep::{SharedAsyncSleep, TokioSleep}; +use aws_sdk_s3::config::retry::{ReconnectMode, RetryConfig}; +use aws_sdk_s3::config::{Credentials, Region, SharedAsyncSleep}; +use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_client::test_connection::wire_mock::{ check_matches, ReplayedEvent, WireLevelTestConnection, }; use aws_smithy_client::{ev, match_events}; -use aws_smithy_types::retry::{ReconnectMode, RetryConfig}; -use aws_types::region::Region; -use aws_types::SdkConfig; #[tokio::test] -/// test that disabling reconnects on retry config disables them for the client -async fn disable_reconnects() { +async fn test_disable_reconnect_on_503() { let mock = WireLevelTestConnection::spinup(vec![ ReplayedEvent::status(503), ReplayedEvent::status(503), @@ -24,9 +20,9 @@ async fn disable_reconnects() { ]) .await; - let sdk_config = SdkConfig::builder() + let config = aws_sdk_s3::Config::builder() .region(Region::from_static("us-east-2")) - .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) + .credentials_provider(Credentials::for_tests()) .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) .endpoint_url(mock.endpoint_url()) .http_connector(mock.http_connector()) @@ -34,7 +30,7 @@ async fn disable_reconnects() { RetryConfig::standard().with_reconnect_mode(ReconnectMode::ReuseAllConnections), ) .build(); - let client = aws_sdk_s3::Client::new(&sdk_config); + let client = aws_sdk_s3::Client::from_conf(config); let resp = client .get_object() .bucket("bucket") @@ -56,7 +52,7 @@ async fn disable_reconnects() { } #[tokio::test] -async fn reconnect_on_503() { +async fn test_enabling_reconnect_on_503() { let mock = WireLevelTestConnection::spinup(vec![ ReplayedEvent::status(503), ReplayedEvent::status(503), @@ -64,15 +60,17 @@ async fn reconnect_on_503() { ]) .await; - let sdk_config = SdkConfig::builder() + let config = aws_sdk_s3::Config::builder() .region(Region::from_static("us-east-2")) - .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) + .credentials_provider(Credentials::for_tests()) .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) .endpoint_url(mock.endpoint_url()) .http_connector(mock.http_connector()) - .retry_config(RetryConfig::standard()) + .retry_config( + RetryConfig::standard().with_reconnect_mode(ReconnectMode::ReconnectOnTransientError), + ) .build(); - let client = aws_sdk_s3::Client::new(&sdk_config); + let client = aws_sdk_s3::Client::from_conf(config); let resp = client .get_object() .bucket("bucket") diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt new file mode 100644 index 00000000000..eaed687aebe --- /dev/null +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ConnectionPoisoningConfigCustomization.kt @@ -0,0 +1,37 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.customizations + +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization +import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection +import software.amazon.smithy.rust.codegen.core.rustlang.Writable +import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.writable +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.smithyRuntime + +class ConnectionPoisoningRuntimePluginCustomization( + codegenContext: ClientCodegenContext, +) : ServiceRuntimePluginCustomization() { + private val runtimeConfig = codegenContext.runtimeConfig + + override fun section(section: ServiceRuntimePluginSection): Writable = writable { + when (section) { + is ServiceRuntimePluginSection.RegisterInterceptor -> { + // This interceptor assumes that a compatible Connector is set. Otherwise, connection poisoning + // won't work and an error message will be logged. + section.registerInterceptor(runtimeConfig, this) { + rust( + "#T::new()", + smithyRuntime(runtimeConfig).resolve("client::connectors::connection_poisoning::ConnectionPoisoningInterceptor"), + ) + } + } + + else -> emptySection + } + } +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt index e4923c0d789..2c80bfd0cb5 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt @@ -40,8 +40,8 @@ private class HttpConnectorConfigCustomization( *preludeScope, "Connection" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::orchestrator::Connection"), "ConnectorSettings" to RuntimeType.smithyClient(runtimeConfig).resolve("http_connector::ConnectorSettings"), - "DynConnection" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::orchestrator::DynConnection"), - "DynConnectorAdapter" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::connections::adapter::DynConnectorAdapter"), + "DynConnector" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::connectors::DynConnector"), + "DynConnectorAdapter" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::connectors::adapter::DynConnectorAdapter"), "HttpConnector" to RuntimeType.smithyClient(runtimeConfig).resolve("http_connector::HttpConnector"), "SharedAsyncSleep" to RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep::SharedAsyncSleep"), "TimeoutConfig" to RuntimeType.smithyTypes(runtimeConfig).resolve("timeout::TimeoutConfig"), @@ -197,14 +197,14 @@ private class HttpConnectorConfigCustomization( let connector_settings = #{ConnectorSettings}::from_timeout_config(&timeout_config); - if let Some(connection) = layer.load::<#{HttpConnector}>() + if let Some(connector) = layer.load::<#{HttpConnector}>() .and_then(|c| c.connector(&connector_settings, sleep_impl.clone())) .or_else(|| #{default_connector}(&connector_settings, sleep_impl)) { - let connection: #{DynConnection} = #{DynConnection}::new(#{DynConnectorAdapter}::new( + let connector: #{DynConnector} = #{DynConnector}::new(#{DynConnectorAdapter}::new( // TODO(enableNewSmithyRuntimeCleanup): Replace the tower-based DynConnector and remove DynConnectorAdapter when deleting the middleware implementation - connection + connector )); - #{ConfigBagAccessors}::set_connection(&mut layer, connection); + #{ConfigBagAccessors}::set_connector(&mut layer, connector); } """, diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 07ece3fb784..1c68cb0e87a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -15,7 +15,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.Writable 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.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope import software.amazon.smithy.rust.codegen.core.smithy.RustCrate @@ -26,8 +25,7 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon private val retryConfig = RuntimeType.smithyTypes(runtimeConfig).resolve("retry") private val sleepModule = RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep") private val timeoutModule = RuntimeType.smithyTypes(runtimeConfig).resolve("timeout") - private val smithyRuntimeCrate = RuntimeType.smithyRuntime(runtimeConfig) - private val retries = smithyRuntimeCrate.resolve("client::retries") + private val retries = RuntimeType.smithyRuntime(runtimeConfig).resolve("client::retries") private val moduleUseName = codegenContext.moduleUseName() private val codegenScope = arrayOf( *preludeScope, @@ -367,7 +365,7 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon } } - ServiceConfig.BuilderBuild -> { + is ServiceConfig.BuilderBuild -> { if (runtimeMode.defaultToOrchestrator) { rustTemplate( """ @@ -420,7 +418,10 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon } } -class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) { +class ResiliencyReExportCustomization(codegenContext: ClientCodegenContext) { + private val runtimeConfig = codegenContext.runtimeConfig + private val runtimeMode = codegenContext.smithyRuntimeMode + fun extras(rustCrate: RustCrate) { rustCrate.withModule(ClientRustModule.config) { rustTemplate( @@ -430,13 +431,16 @@ class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) } rustCrate.withModule(ClientRustModule.Config.retry) { rustTemplate( - "pub use #{types_retry}::{RetryConfig, RetryConfigBuilder, RetryMode};", + "pub use #{types_retry}::{RetryConfig, RetryConfigBuilder, RetryMode, ReconnectMode};", "types_retry" to RuntimeType.smithyTypes(runtimeConfig).resolve("retry"), ) - rustTemplate( - "pub use #{RetryPartition};", - "RetryPartition" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::retries::RetryPartition"), - ) + + if (runtimeMode.generateOrchestrator) { + rustTemplate( + "pub use #{types_retry}::RetryPartition;", + "types_retry" to RuntimeType.smithyRuntime(runtimeConfig).resolve("client::retries"), + ) + } } rustCrate.withModule(ClientRustModule.Config.timeout) { rustTemplate( @@ -449,30 +453,34 @@ class ResiliencyReExportCustomization(private val runtimeConfig: RuntimeConfig) class ResiliencyServiceRuntimePluginCustomization(codegenContext: ClientCodegenContext) : ServiceRuntimePluginCustomization() { private val runtimeConfig = codegenContext.runtimeConfig - private val smithyRuntimeCrate = RuntimeType.smithyRuntime(runtimeConfig) - private val retries = smithyRuntimeCrate.resolve("client::retries") + private val runtimeMode = codegenContext.smithyRuntimeMode + private val smithyRuntime = RuntimeType.smithyRuntime(runtimeConfig) + private val retries = smithyRuntime.resolve("client::retries") private val codegenScope = arrayOf( "TokenBucket" to retries.resolve("TokenBucket"), "TokenBucketPartition" to retries.resolve("TokenBucketPartition"), "ClientRateLimiter" to retries.resolve("ClientRateLimiter"), "ClientRateLimiterPartition" to retries.resolve("ClientRateLimiterPartition"), - "StaticPartitionMap" to smithyRuntimeCrate.resolve("static_partition_map::StaticPartitionMap"), + "StaticPartitionMap" to smithyRuntime.resolve("static_partition_map::StaticPartitionMap"), ) override fun section(section: ServiceRuntimePluginSection): Writable = writable { - when (section) { - is ServiceRuntimePluginSection.DeclareSingletons -> { - // TODO(enableNewSmithyRuntimeCleanup) We can use the standard library's `OnceCell` once we upgrade the - // MSRV to 1.70 - rustTemplate( - """ - static TOKEN_BUCKET: #{StaticPartitionMap}<#{TokenBucketPartition}, #{TokenBucket}> = #{StaticPartitionMap}::new(); - static CLIENT_RATE_LIMITER: #{StaticPartitionMap}<#{ClientRateLimiterPartition}, #{ClientRateLimiter}> = #{StaticPartitionMap}::new(); - """, - *codegenScope, - ) + if (runtimeMode.generateOrchestrator) { + when (section) { + is ServiceRuntimePluginSection.DeclareSingletons -> { + // TODO(enableNewSmithyRuntimeCleanup) We can use the standard library's `OnceCell` once we upgrade the + // MSRV to 1.70 + rustTemplate( + """ + static TOKEN_BUCKET: #{StaticPartitionMap}<#{TokenBucketPartition}, #{TokenBucket}> = #{StaticPartitionMap}::new(); + static CLIENT_RATE_LIMITER: #{StaticPartitionMap}<#{ClientRateLimiterPartition}, #{ClientRateLimiter}> = #{StaticPartitionMap}::new(); + """, + *codegenScope, + ) + } + + else -> emptySection } - else -> emptySection } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt index 4b85a6bb533..e26e228c7e6 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.customize import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule +import software.amazon.smithy.rust.codegen.client.smithy.customizations.ConnectionPoisoningRuntimePluginCustomization import software.amazon.smithy.rust.codegen.client.smithy.customizations.EndpointPrefixGenerator import software.amazon.smithy.rust.codegen.client.smithy.customizations.HttpChecksumRequiredGenerator import software.amazon.smithy.rust.codegen.client.smithy.customizations.HttpVersionListCustomization @@ -82,7 +83,7 @@ class RequiredCustomizations : ClientCodegenDecorator { rustCrate.mergeFeature(TestUtilFeature) // Re-export resiliency types - ResiliencyReExportCustomization(codegenContext.runtimeConfig).extras(rustCrate) + ResiliencyReExportCustomization(codegenContext).extras(rustCrate) rustCrate.withModule(ClientRustModule.Primitives) { pubUseSmithyPrimitives(codegenContext, codegenContext.model)(this) @@ -102,7 +103,9 @@ class RequiredCustomizations : ClientCodegenDecorator { codegenContext: ClientCodegenContext, baseCustomizations: List, ): List = if (codegenContext.smithyRuntimeMode.generateOrchestrator) { - baseCustomizations + ResiliencyServiceRuntimePluginCustomization(codegenContext) + baseCustomizations + + ResiliencyServiceRuntimePluginCustomization(codegenContext) + + ConnectionPoisoningRuntimePluginCustomization(codegenContext) } else { baseCustomizations } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index b8023cd6c4a..c77e95940a8 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -67,13 +67,12 @@ sealed class ServiceRuntimePluginSection(name: String) : Section(name) { data class RegisterInterceptor(val interceptorRegistrarName: String) : ServiceRuntimePluginSection("RegisterInterceptor") { /** Generates the code to register an interceptor */ fun registerInterceptor(runtimeConfig: RuntimeConfig, writer: RustWriter, interceptor: Writable) { - val smithyRuntimeApi = RuntimeType.smithyRuntimeApi(runtimeConfig) writer.rustTemplate( """ $interceptorRegistrarName.register(#{SharedInterceptor}::new(#{interceptor}) as _); """, "interceptor" to interceptor, - "SharedInterceptor" to smithyRuntimeApi.resolve("client::interceptors::SharedInterceptor"), + "SharedInterceptor" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::SharedInterceptor"), ) } } diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomizationTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomizationTest.kt index 4f9c1e23e3e..b84f204c240 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomizationTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomizationTest.kt @@ -40,7 +40,7 @@ internal class ResiliencyConfigCustomizationTest { val codegenContext = testClientCodegenContext(model, settings = project.clientRustSettings()) stubConfigProject(codegenContext, ResiliencyConfigCustomization(codegenContext), project) - ResiliencyReExportCustomization(codegenContext.runtimeConfig).extras(project) + ResiliencyReExportCustomization(codegenContext).extras(project) project.compileAndTest() } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt index 6fc9ca7adf2..3cb637b64b8 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.core.rustlang import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.derive +import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.serde import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.util.dq diff --git a/rust-runtime/aws-smithy-runtime-api/src/client.rs b/rust-runtime/aws-smithy-runtime-api/src/client.rs index fec60cf8928..cdcfb847a5e 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client.rs @@ -30,3 +30,6 @@ pub mod auth; /// A type to track the number of requests sent by the orchestrator for a given operation. pub mod request_attempts; + +/// Smithy connectors and related code. +pub mod connectors; diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/config_bag_accessors.rs b/rust-runtime/aws-smithy-runtime-api/src/client/config_bag_accessors.rs index 3b9262f8898..54832acf808 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/config_bag_accessors.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/config_bag_accessors.rs @@ -7,13 +7,13 @@ use crate::client::auth::{ AuthOptionResolver, AuthOptionResolverParams, AuthSchemeId, DynAuthOptionResolver, SharedHttpAuthScheme, }; +use crate::client::connectors::{Connector, DynConnector}; use crate::client::identity::{ ConfiguredIdentityResolver, IdentityResolvers, SharedIdentityResolver, }; use crate::client::orchestrator::{ - Connection, DynConnection, DynEndpointResolver, DynResponseDeserializer, EndpointResolver, - EndpointResolverParams, LoadedRequestBody, ResponseDeserializer, SharedRequestSerializer, - NOT_NEEDED, + DynEndpointResolver, DynResponseDeserializer, EndpointResolver, EndpointResolverParams, + LoadedRequestBody, ResponseDeserializer, SharedRequestSerializer, NOT_NEEDED, }; use crate::client::retries::{DynRetryStrategy, RetryClassifiers, RetryStrategy}; use aws_smithy_async::rt::sleep::SharedAsyncSleep; @@ -133,6 +133,7 @@ mod internal { } } } + use internal::{CloneableSettable, Gettable, Settable}; pub trait ConfigBagAccessors { @@ -219,18 +220,18 @@ pub trait ConfigBagAccessors { )); } - fn connection(&self) -> &dyn Connection + fn connector(&self) -> &dyn Connector where Self: Gettable, { - self.load::().expect("missing connector") + self.load::().expect("missing connector") } - fn set_connection(&mut self, connection: DynConnection) + fn set_connector(&mut self, connection: DynConnector) where Self: Settable, { - self.store_put::(connection); + self.store_put::(connection); } /// Returns the configured HTTP auth schemes. diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/connectors.rs b/rust-runtime/aws-smithy-runtime-api/src/client/connectors.rs new file mode 100644 index 00000000000..fd63eea38c9 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/src/client/connectors.rs @@ -0,0 +1,31 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use crate::client::orchestrator::{BoxFuture, HttpRequest, HttpResponse}; +use aws_smithy_types::config_bag::{Storable, StoreReplace}; +use std::fmt; + +pub trait Connector: Send + Sync + fmt::Debug { + fn call(&self, request: HttpRequest) -> BoxFuture; +} + +#[derive(Debug)] +pub struct DynConnector(Box); + +impl DynConnector { + pub fn new(connection: impl Connector + 'static) -> Self { + Self(Box::new(connection)) + } +} + +impl Connector for DynConnector { + fn call(&self, request: HttpRequest) -> BoxFuture { + (*self.0).call(request) + } +} + +impl Storable for DynConnector { + type Storer = StoreReplace; +} diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context/wrappers.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context/wrappers.rs index 1094ce4ab24..7a79c134279 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context/wrappers.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/context/wrappers.rs @@ -158,6 +158,16 @@ declare_wrapper!( (response: Response) ); +impl<'a, I, O, E: Debug> BeforeDeserializationInterceptorContextMut<'a, I, O, E> { + #[doc(hidden)] + /// Downgrade this helper struct, returning the underlying InterceptorContext. There's no good + /// reason to use this unless you're writing tests or you have to interact with an API that + /// doesn't support the helper structs. + pub fn into_inner(&mut self) -> &'_ mut InterceptorContext { + self.inner + } +} + declare_wrapper!( (AfterDeserializationInterceptorContextRef readonly) (input: I) diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs index 6bcb8f0431f..9384509900f 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/orchestrator.rs @@ -12,7 +12,6 @@ use aws_smithy_types::endpoint::Endpoint; use aws_smithy_types::type_erasure::{TypeErasedBox, TypedBox}; use bytes::Bytes; use std::fmt; -use std::fmt::Debug; use std::future::Future as StdFuture; use std::pin::Pin; use std::sync::Arc; @@ -94,29 +93,6 @@ impl Storable for DynResponseDeserializer { type Storer = StoreReplace; } -pub trait Connection: Send + Sync + fmt::Debug { - fn call(&self, request: HttpRequest) -> BoxFuture; -} - -#[derive(Debug)] -pub struct DynConnection(Box); - -impl DynConnection { - pub fn new(connection: impl Connection + 'static) -> Self { - Self(Box::new(connection)) - } -} - -impl Connection for DynConnection { - fn call(&self, request: HttpRequest) -> BoxFuture { - (*self.0).call(request) - } -} - -impl Storable for DynConnection { - type Storer = StoreReplace; -} - #[derive(Debug)] pub struct EndpointResolverParams(TypeErasedBox); diff --git a/rust-runtime/aws-smithy-runtime/src/client.rs b/rust-runtime/aws-smithy-runtime/src/client.rs index 6bf53974892..cb0d8f1ae86 100644 --- a/rust-runtime/aws-smithy-runtime/src/client.rs +++ b/rust-runtime/aws-smithy-runtime/src/client.rs @@ -5,8 +5,14 @@ pub mod auth; -/// Smithy connector runtime plugins -pub mod connections; +/// Smithy code related to connectors and connections. +/// +/// A "connector" manages one or more "connections", handles connection timeouts, re-establishes +/// connections, etc. +/// +/// "Connections" refers to the actual transport layer implementation of the connector. +/// By default, the orchestrator uses a connector provided by `hyper`. +pub mod connectors; /// The client orchestrator implementation pub mod orchestrator; diff --git a/rust-runtime/aws-smithy-runtime/src/client/connections.rs b/rust-runtime/aws-smithy-runtime/src/client/connectors.rs similarity index 77% rename from rust-runtime/aws-smithy-runtime/src/client/connections.rs rename to rust-runtime/aws-smithy-runtime/src/client/connectors.rs index e05eedf2e80..60e9a770969 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/connections.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/connectors.rs @@ -3,14 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ +pub mod connection_poisoning; #[cfg(feature = "test-util")] -pub mod test_connection; +pub mod test_util; pub mod adapter { use aws_smithy_client::erase::DynConnector; - use aws_smithy_runtime_api::client::orchestrator::{ - BoxFuture, Connection, HttpRequest, HttpResponse, - }; + use aws_smithy_runtime_api::client::connectors::Connector; + use aws_smithy_runtime_api::client::orchestrator::{BoxFuture, HttpRequest, HttpResponse}; use std::sync::{Arc, Mutex}; #[derive(Debug)] @@ -27,7 +27,7 @@ pub mod adapter { } } - impl Connection for DynConnectorAdapter { + impl Connector for DynConnectorAdapter { fn call(&self, request: HttpRequest) -> BoxFuture { let future = self.dyn_connector.lock().unwrap().call_lite(request); future diff --git a/rust-runtime/aws-smithy-runtime/src/client/connectors/connection_poisoning.rs b/rust-runtime/aws-smithy-runtime/src/client/connectors/connection_poisoning.rs new file mode 100644 index 00000000000..e2d08a60d53 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime/src/client/connectors/connection_poisoning.rs @@ -0,0 +1,132 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use aws_smithy_http::connection::{CaptureSmithyConnection, ConnectionMetadata}; +use aws_smithy_runtime_api::box_error::BoxError; +use aws_smithy_runtime_api::client::config_bag_accessors::ConfigBagAccessors; +use aws_smithy_runtime_api::client::interceptors::context::{ + BeforeDeserializationInterceptorContextMut, BeforeTransmitInterceptorContextMut, +}; +use aws_smithy_runtime_api::client::interceptors::Interceptor; +use aws_smithy_runtime_api::client::retries::{ClassifyRetry, RetryReason}; +use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreReplace}; +use aws_smithy_types::retry::{ErrorKind, ReconnectMode, RetryConfig}; +use std::fmt; +use tracing::{debug, error}; + +/// A interceptor for poisoning connections in response to certain events. +/// +/// This interceptor, when paired with a compatible connection, allows the connection to be +/// poisoned in reaction to certain events *(like receiving a transient error.)* This allows users +/// to avoid sending requests to a server that isn't responding. This can increase the load on a +/// server, because more connections will be made overall. +/// +/// **In order for this interceptor to work,** the configured connection must interact with the +/// "connection retriever" stored in an HTTP request's `extensions` map. For an example of this, +/// see [aws_smithy_client::hyper_ext::Adapter](https://github.com/awslabs/smithy-rs/blob/47b3d23ff3cabd67e797af616101f5a4ea6be5e8/rust-runtime/aws-smithy-client/src/hyper_ext.rs#L155). +/// When a connection is made available to the retriever, this interceptor will call a `.poison` +/// method on it, signalling that the connection should be dropped. It is up to the connection +/// implementer to handle this. +#[non_exhaustive] +#[derive(Debug, Default)] +pub struct ConnectionPoisoningInterceptor {} + +impl ConnectionPoisoningInterceptor { + pub fn new() -> Self { + Self::default() + } +} + +impl Interceptor for ConnectionPoisoningInterceptor { + fn modify_before_transmit( + &self, + context: &mut BeforeTransmitInterceptorContextMut<'_>, + cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + let capture_smithy_connection = CaptureSmithyConnectionWrapper::new(); + context + .request_mut() + .extensions_mut() + .insert(capture_smithy_connection.clone_inner()); + cfg.interceptor_state().store_put(capture_smithy_connection); + + Ok(()) + } + + fn modify_before_deserialization( + &self, + context: &mut BeforeDeserializationInterceptorContextMut<'_>, + cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + let reconnect_mode = cfg + .load::() + .map(RetryConfig::reconnect_mode) + .unwrap_or(ReconnectMode::ReconnectOnTransientError); + let captured_connection = cfg.load::().cloned(); + let retry_classifiers = cfg.retry_classifiers(); + + let error_is_transient = retry_classifiers + .classify_retry(context.into_inner()) + .map(|reason| reason == RetryReason::Error(ErrorKind::TransientError)) + .unwrap_or_default(); + let connection_poisoning_is_enabled = + reconnect_mode == ReconnectMode::ReconnectOnTransientError; + + if error_is_transient && connection_poisoning_is_enabled { + debug!("received a transient error, poisoning the connection..."); + + if let Some(captured_connection) = captured_connection.and_then(|conn| conn.get()) { + captured_connection.poison(); + debug!("the connection was poisoned") + } else { + error!( + "unable to poison the connection because no connection was found! The underlying HTTP connector never set a connection." + ); + } + } + + Ok(()) + } +} + +// TODO(enableNewSmithyRuntimeLaunch) We won't need this once we absorb aws_smithy_http into the +// new runtime crate. +#[derive(Clone, Default)] +pub struct CaptureSmithyConnectionWrapper { + inner: CaptureSmithyConnection, +} + +impl CaptureSmithyConnectionWrapper { + pub fn new() -> Self { + Self { + inner: CaptureSmithyConnection::new(), + } + } + + pub fn clone_inner(&self) -> CaptureSmithyConnection { + self.inner.clone() + } + + pub fn get(&self) -> Option { + self.inner.get() + } + + pub fn set_connection_retriever(&self, f: F) + where + F: Fn() -> Option + Send + Sync + 'static, + { + self.inner.set_connection_retriever(f) + } +} + +impl Storable for CaptureSmithyConnectionWrapper { + type Storer = StoreReplace; +} + +impl fmt::Debug for CaptureSmithyConnectionWrapper { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "CaptureSmithyConnectionWrapper") + } +} diff --git a/rust-runtime/aws-smithy-runtime/src/client/connections/test_connection.rs b/rust-runtime/aws-smithy-runtime/src/client/connectors/test_util.rs similarity index 96% rename from rust-runtime/aws-smithy-runtime/src/client/connections/test_connection.rs rename to rust-runtime/aws-smithy-runtime/src/client/connectors/test_util.rs index 7db7ba541ce..5a9b3bfdfd8 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/connections/test_connection.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/connectors/test_util.rs @@ -9,9 +9,8 @@ use aws_smithy_async::rt::sleep::{AsyncSleep, SharedAsyncSleep}; use aws_smithy_http::body::SdkBody; use aws_smithy_http::result::ConnectorError; use aws_smithy_protocol_test::{assert_ok, validate_body, MediaType}; -use aws_smithy_runtime_api::client::orchestrator::{ - BoxFuture, Connection, HttpRequest, HttpResponse, -}; +use aws_smithy_runtime_api::client::connectors::Connector; +use aws_smithy_runtime_api::client::orchestrator::{BoxFuture, HttpRequest, HttpResponse}; use http::header::{HeaderName, CONTENT_TYPE}; use std::fmt::Debug; use std::ops::Deref; @@ -182,7 +181,7 @@ impl ValidateRequest { } } -/// TestConnection for use as a [`Connection`]. +/// TestConnection for use as a [`Connector`]. /// /// A basic test connection. It will: /// - Respond to requests with a preloaded series of responses @@ -223,7 +222,7 @@ impl TestConnection { } } -impl Connection for TestConnection { +impl Connector for TestConnection { fn call(&self, request: HttpRequest) -> BoxFuture { let (res, simulated_latency) = if let Some(event) = self.data.lock().unwrap().pop() { self.requests.lock().unwrap().push(ValidateRequest { diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index 6c950b81d54..de6f9dceaad 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -281,7 +281,7 @@ async fn try_attempt( ctx.enter_transmit_phase(); let call_result = halt_on_err!([ctx] => { let request = ctx.take_request().expect("set during serialization"); - cfg.connection().call(request).await.map_err(|err| { + cfg.connector().call(request).await.map_err(|err| { match err.downcast() { Ok(connector_error) => OrchestratorError::connector(*connector_error), Err(box_err) => OrchestratorError::other(box_err) @@ -349,6 +349,7 @@ mod tests { serializer::CannedRequestSerializer, }; use ::http::{Request, Response, StatusCode}; + use aws_smithy_runtime_api::client::connectors::Connector; use aws_smithy_runtime_api::client::interceptors::context::{ AfterDeserializationInterceptorContextRef, BeforeDeserializationInterceptorContextMut, BeforeDeserializationInterceptorContextRef, BeforeSerializationInterceptorContextMut, @@ -360,7 +361,7 @@ mod tests { Interceptor, InterceptorRegistrar, SharedInterceptor, }; use aws_smithy_runtime_api::client::orchestrator::{ - DynConnection, DynEndpointResolver, DynResponseDeserializer, SharedRequestSerializer, + DynConnector, DynEndpointResolver, DynResponseDeserializer, SharedRequestSerializer, }; use aws_smithy_runtime_api::client::retries::DynRetryStrategy; use aws_smithy_runtime_api::client::runtime_plugin::{RuntimePlugin, RuntimePlugins}; @@ -388,6 +389,24 @@ mod tests { ) } + #[derive(Debug, Default)] + pub struct OkConnector {} + + impl OkConnector { + pub fn new() -> Self { + Self::default() + } + } + + impl Connector for OkConnector { + fn call(&self, _request: HttpRequest) -> BoxFuture { + Box::pin(Future::ready(Ok(http::Response::builder() + .status(200) + .body(SdkBody::empty()) + .expect("OK response is valid")))) + } + } + #[derive(Debug)] struct TestOperationRuntimePlugin; @@ -403,7 +422,7 @@ mod tests { StaticUriEndpointResolver::http_localhost(8080), )); cfg.set_endpoint_resolver_params(StaticUriEndpointResolverParams::new().into()); - cfg.set_connection(DynConnection::new(OkConnector::new())); + cfg.set_connector(DynConnector::new(OkConnector::new())); Some(cfg.freeze()) } diff --git a/rust-runtime/aws-smithy-runtime/src/client/test_util.rs b/rust-runtime/aws-smithy-runtime/src/client/test_util.rs index 101c971eff3..de7aef4ac5e 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/test_util.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/test_util.rs @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -pub mod connector; pub mod deserializer; pub mod interceptors; pub mod serializer; diff --git a/rust-runtime/aws-smithy-runtime/src/client/test_util/connector.rs b/rust-runtime/aws-smithy-runtime/src/client/test_util/connector.rs deleted file mode 100644 index 73507bf9fbf..00000000000 --- a/rust-runtime/aws-smithy-runtime/src/client/test_util/connector.rs +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use aws_smithy_http::body::SdkBody; -use aws_smithy_runtime_api::client::orchestrator::{ - BoxFuture, Connection, Future, HttpRequest, HttpResponse, -}; - -#[derive(Debug, Default)] -pub struct OkConnector {} - -impl OkConnector { - pub fn new() -> Self { - Self::default() - } -} - -impl Connection for OkConnector { - fn call(&self, _request: HttpRequest) -> BoxFuture { - Box::pin(Future::ready(Ok(http::Response::builder() - .status(200) - .body(SdkBody::empty()) - .expect("OK response is valid")))) - } -} diff --git a/rust-runtime/aws-smithy-types/src/retry.rs b/rust-runtime/aws-smithy-types/src/retry.rs index a5144178142..a2866b84c10 100644 --- a/rust-runtime/aws-smithy-types/src/retry.rs +++ b/rust-runtime/aws-smithy-types/src/retry.rs @@ -301,6 +301,10 @@ pub enum ReconnectMode { ReuseAllConnections, } +impl Storable for ReconnectMode { + type Storer = StoreReplace; +} + impl RetryConfig { /// Creates a default `RetryConfig` with `RetryMode::Standard` and max attempts of three. pub fn standard() -> Self { From 847ed0b66afe6eca3c64a764a4e7c7c16b0851df Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 30 Jun 2023 14:21:15 -0700 Subject: [PATCH 4/6] Fix the S3 alternate runtime retries test in orchestrator mode (#2825) This PR fixes the S3 `alternative-async-runtime` retry tests. The retry backoff time was being included in the operation attempt timeout, which I think is undesirable as it makes retry config much harder to get right since the backoff times are not predictable (they have randomness incorporated into them). The overall operation timeout is the only one that needs to incorporate backoff times. In addition, this PR also: - Updates READMEs for the `aws-smithy-runtime-api` and `aws-smithy-runtime` crates - Adds top-level crate docs to describe features to `aws-smithy-runtime` - Copies `capture_test_logs` into `aws-smithy-runtime` so that it can be used (just about) everywhere instead of just in `aws-config` - Adds service/operation name to the tracing `invoke` span so it's possible to tell which operation the events are for - Makes the `Debug` impl for `Identity` useful - Adds a ton of trace/debug spans and events to the orchestrator - Fixes an issue in `aws-smithy-runtime` where a huge amount of the orchestrator tests weren't being compiled due to a removed feature flag ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- aws/rust-runtime/aws-config/src/test_case.rs | 2 + .../retries-with-client-rate-limiting.rs | 53 +++---- aws/sdk/integration-tests/s3/Cargo.toml | 1 + .../s3/tests/alternative-async-runtime.rs | 17 ++- .../ResiliencyConfigCustomization.kt | 5 +- .../smithy/generators/OperationGenerator.kt | 10 +- .../OperationRuntimePluginGenerator.kt | 1 + .../protocol/MakeOperationGenerator.kt | 7 +- .../smithy/rust/codegen/core/util/Smithy.kt | 5 + rust-runtime/aws-smithy-runtime-api/README.md | 4 +- .../src/client/identity.rs | 22 ++- .../src/client/interceptors.rs | 11 ++ .../src/client/runtime_plugin.rs | 2 + rust-runtime/aws-smithy-runtime/Cargo.toml | 9 +- rust-runtime/aws-smithy-runtime/README.md | 4 +- .../src/client/orchestrator.rs | 138 ++++++++++++------ .../src/client/orchestrator/auth.rs | 14 +- .../src/client/orchestrator/endpoints.rs | 3 + .../src/client/retries/strategy/standard.rs | 2 +- rust-runtime/aws-smithy-runtime/src/lib.rs | 15 +- .../aws-smithy-runtime/src/test_util.rs | 7 + .../src/test_util/capture_test_logs.rs | 90 ++++++++++++ .../check-aws-sdk-orchestrator-impl | 14 -- 23 files changed, 318 insertions(+), 118 deletions(-) create mode 100644 rust-runtime/aws-smithy-runtime/src/test_util.rs create mode 100644 rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs index 8392a92a169..14859d57ef2 100644 --- a/aws/rust-runtime/aws-config/src/test_case.rs +++ b/aws/rust-runtime/aws-config/src/test_case.rs @@ -136,6 +136,8 @@ pub(crate) struct Metadata { name: String, } +// TODO(enableNewSmithyRuntimeCleanup): Replace Tee, capture_test_logs, and Rx with +// the implementations added to aws_smithy_runtime::test_util::capture_test_logs struct Tee { buf: Arc>>, quiet: bool, diff --git a/aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs b/aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs index d3f05db8656..21ad0470bc8 100644 --- a/aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs +++ b/aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs @@ -7,16 +7,13 @@ mod test { use aws_sdk_dynamodb::config::{Credentials, Region, SharedAsyncSleep}; use aws_sdk_dynamodb::{config::retry::RetryConfig, error::ProvideErrorMetadata}; - use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_async::test_util::instant_time_and_sleep; use aws_smithy_async::time::SharedTimeSource; - use aws_smithy_async::time::SystemTimeSource; use aws_smithy_client::test_connection::TestConnection; use aws_smithy_http::body::SdkBody; use aws_smithy_runtime::client::retries::RetryPartition; use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; - use aws_smithy_types::timeout::TimeoutConfigBuilder; - use std::time::{Duration, Instant, SystemTime}; + use std::time::{Duration, SystemTime}; fn req() -> HttpRequest { http::Request::builder() @@ -111,15 +108,16 @@ mod test { } #[tokio::test] - async fn test_adaptive_retries_with_throttling_errors_times_out() { - tracing_subscriber::fmt::init(); + async fn test_adaptive_retries_with_throttling_errors() { + let (time_source, sleep_impl) = instant_time_and_sleep(SystemTime::UNIX_EPOCH); + let events = vec![ // First operation - (req(), err()), + (req(), throttling_err()), + (req(), throttling_err()), (req(), ok()), // Second operation (req(), err()), - (req(), throttling_err()), (req(), ok()), ]; @@ -130,44 +128,31 @@ mod test { .retry_config( RetryConfig::adaptive() .with_max_attempts(4) - .with_initial_backoff(Duration::from_millis(50)) .with_use_static_exponential_base(true), ) - .timeout_config( - TimeoutConfigBuilder::new() - .operation_attempt_timeout(Duration::from_millis(100)) - .build(), - ) - .time_source(SharedTimeSource::new(SystemTimeSource::new())) - .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .http_connector(conn.clone()) + .time_source(SharedTimeSource::new(time_source)) + .sleep_impl(SharedAsyncSleep::new(sleep_impl.clone())) .retry_partition(RetryPartition::new( - "test_adaptive_retries_with_throttling_errors_times_out", + "test_adaptive_retries_with_throttling_errors", )) + .http_connector(conn.clone()) .build(); - let expected_table_names = vec!["Test".to_owned()]; - let start = Instant::now(); // We create a new client each time to ensure that the cross-client retry state is working. let client = aws_sdk_dynamodb::Client::from_conf(config.clone()); let res = client.list_tables().send().await.unwrap(); + assert_eq!(sleep_impl.total_duration(), Duration::from_secs(40)); assert_eq!(res.table_names(), Some(expected_table_names.as_slice())); // Three requests should have been made, two failing & one success - assert_eq!(conn.requests().len(), 2); + assert_eq!(conn.requests().len(), 3); - let client = aws_sdk_dynamodb::Client::from_conf(config); - let err = client.list_tables().send().await.unwrap_err(); - assert_eq!(err.to_string(), "request has timed out".to_owned()); - // two requests should have been made, both failing (plus previous requests) - assert_eq!(conn.requests().len(), 2 + 2); - - let since = start.elapsed(); - // At least 300 milliseconds must pass: - // - 50ms for the first retry on attempt 1 - // - 50ms for the second retry on attempt 3 - // - 100ms for the throttling delay triggered by attempt 4, which required a delay longer than the attempt timeout. - // - 100ms for the 5th attempt, which would have succeeded, but required a delay longer than the attempt timeout. - assert!(since.as_secs_f64() > 0.3); + let client = aws_sdk_dynamodb::Client::from_conf(config.clone()); + let res = client.list_tables().send().await.unwrap(); + assert!(Duration::from_secs(48) < sleep_impl.total_duration()); + assert!(Duration::from_secs(49) > sleep_impl.total_duration()); + assert_eq!(res.table_names(), Some(expected_table_names.as_slice())); + // Two requests should have been made, one failing & one success (plus previous requests) + assert_eq!(conn.requests().len(), 5); } } diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 1dfcc942576..8a27b1cd2c7 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -21,6 +21,7 @@ aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async", features aws-smithy-client = { path = "../../build/aws-sdk/sdk/aws-smithy-client", features = ["test-util", "rustls"] } aws-smithy-http = { path = "../../build/aws-sdk/sdk/aws-smithy-http" } aws-smithy-protocol-test = { path = "../../build/aws-sdk/sdk/aws-smithy-protocol-test" } +aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util"] } aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" } aws-types = { path = "../../build/aws-sdk/sdk/aws-types" } bytes = "1" diff --git a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs index db36cfeb92f..8d1a604d95d 100644 --- a/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs +++ b/aws/sdk/integration-tests/s3/tests/alternative-async-runtime.rs @@ -20,6 +20,9 @@ use aws_smithy_types::timeout::TimeoutConfig; use std::fmt::Debug; use std::time::{Duration, Instant}; +#[cfg(aws_sdk_orchestrator_mode)] +use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; + #[derive(Debug)] struct SmolSleep; @@ -33,6 +36,9 @@ impl AsyncSleep for SmolSleep { #[test] fn test_smol_runtime_timeouts() { + #[cfg(aws_sdk_orchestrator_mode)] + let _guard = capture_test_logs(); + if let Err(err) = smol::block_on(async { timeout_test(SharedAsyncSleep::new(SmolSleep)).await }) { println!("{err}"); @@ -42,6 +48,9 @@ fn test_smol_runtime_timeouts() { #[test] fn test_smol_runtime_retry() { + #[cfg(aws_sdk_orchestrator_mode)] + let _guard = capture_test_logs(); + if let Err(err) = smol::block_on(async { retry_test(SharedAsyncSleep::new(SmolSleep)).await }) { println!("{err}"); panic!(); @@ -59,6 +68,9 @@ impl AsyncSleep for AsyncStdSleep { #[test] fn test_async_std_runtime_timeouts() { + #[cfg(aws_sdk_orchestrator_mode)] + let _guard = capture_test_logs(); + if let Err(err) = async_std::task::block_on(async { timeout_test(SharedAsyncSleep::new(AsyncStdSleep)).await }) { @@ -69,6 +81,9 @@ fn test_async_std_runtime_timeouts() { #[test] fn test_async_std_runtime_retry() { + #[cfg(aws_sdk_orchestrator_mode)] + let _guard = capture_test_logs(); + if let Err(err) = async_std::task::block_on(async { retry_test(SharedAsyncSleep::new(AsyncStdSleep)).await }) { @@ -137,7 +152,7 @@ async fn retry_test(sleep_impl: SharedAsyncSleep) -> Result<(), Box().cloned().unwrap_or_else(|| #{RetryPartition}::new("${codegenContext.serviceShape.id.name}")); + let retry_partition = layer.load::<#{RetryPartition}>().cloned().unwrap_or_else(|| #{RetryPartition}::new("${codegenContext.serviceShape.sdkId()}")); let retry_config = layer.load::<#{RetryConfig}>().cloned().unwrap_or_else(#{RetryConfig}::disabled); if retry_config.has_retry() { - #{debug}!("creating retry strategy with partition '{}'", retry_partition); + #{debug}!("using retry strategy with partition '{}'", retry_partition); } if retry_config.mode() == #{RetryMode}::Adaptive { diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt index b59ff1c3395..5bf9c8615b5 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt @@ -27,8 +27,10 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.pre import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations import software.amazon.smithy.rust.codegen.core.smithy.generators.protocol.ProtocolPayloadGenerator import software.amazon.smithy.rust.codegen.core.smithy.protocols.Protocol +import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.inputShape import software.amazon.smithy.rust.codegen.core.util.outputShape +import software.amazon.smithy.rust.codegen.core.util.sdkId open class OperationGenerator( private val codegenContext: ClientCodegenContext, @@ -142,7 +144,13 @@ open class OperationGenerator( stop_point: #{StopPoint}, ) -> #{Result}<#{InterceptorContext}, #{SdkError}<#{Error}, #{HttpResponse}>> { let input = #{TypedBox}::new(input).erase(); - #{invoke_with_stop_point}(input, runtime_plugins, stop_point).await + #{invoke_with_stop_point}( + ${codegenContext.serviceShape.sdkId().dq()}, + ${operationName.dq()}, + input, + runtime_plugins, + stop_point + ).await } pub(crate) fn operation_runtime_plugins( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt index 5edfdedb040..febdb958256 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationRuntimePluginGenerator.kt @@ -64,6 +64,7 @@ class OperationRuntimePluginGenerator( fn config(&self) -> #{Option}<#{FrozenLayer}> { let mut cfg = #{Layer}::new(${operationShape.id.name.dq()}); use #{ConfigBagAccessors} as _; + cfg.set_request_serializer(#{SharedRequestSerializer}::new(${operationStructName}RequestSerializer)); cfg.set_response_deserializer(#{DynResponseDeserializer}::new(${operationStructName}ResponseDeserializer)); diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/MakeOperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/MakeOperationGenerator.kt index fdb92dd3f70..d79817df40c 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/MakeOperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/MakeOperationGenerator.kt @@ -5,7 +5,6 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators.protocol -import software.amazon.smithy.aws.traits.ServiceTrait import software.amazon.smithy.model.shapes.BlobShape import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule @@ -31,9 +30,9 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.HttpLocation import software.amazon.smithy.rust.codegen.core.smithy.protocols.Protocol import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.findStreamingMember -import software.amazon.smithy.rust.codegen.core.util.getTrait import software.amazon.smithy.rust.codegen.core.util.inputShape import software.amazon.smithy.rust.codegen.core.util.letIf +import software.amazon.smithy.rust.codegen.core.util.sdkId // TODO(enableNewSmithyRuntimeCleanup): Delete this class when cleaning up `enableNewSmithyRuntime` /** Generates the `make_operation` function on input structs */ @@ -53,9 +52,7 @@ open class MakeOperationGenerator( private val defaultClassifier = RuntimeType.smithyHttp(runtimeConfig) .resolve("retry::DefaultResponseRetryClassifier") - private val sdkId = - codegenContext.serviceShape.getTrait()?.sdkId?.lowercase()?.replace(" ", "") - ?: codegenContext.serviceShape.id.getName(codegenContext.serviceShape) + private val sdkId = codegenContext.serviceShape.sdkId() private val codegenScope = arrayOf( *preludeScope, diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt index 299cdff24d2..bde5c4b3389 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt @@ -5,6 +5,7 @@ package software.amazon.smithy.rust.codegen.core.util +import software.amazon.smithy.aws.traits.ServiceTrait import software.amazon.smithy.codegen.core.CodegenException import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.BooleanShape @@ -146,3 +147,7 @@ fun String.shapeId() = ShapeId.from(this) /** Returns the service name, or a default value if the service doesn't have a title trait */ fun ServiceShape.serviceNameOrDefault(default: String) = getTrait()?.value ?: default + +/** Returns the SDK ID of the given service shape */ +fun ServiceShape.sdkId(): String = + getTrait()?.sdkId?.lowercase()?.replace(" ", "") ?: id.getName(this) diff --git a/rust-runtime/aws-smithy-runtime-api/README.md b/rust-runtime/aws-smithy-runtime-api/README.md index 0e336610215..7eb00c40dc7 100644 --- a/rust-runtime/aws-smithy-runtime-api/README.md +++ b/rust-runtime/aws-smithy-runtime-api/README.md @@ -1,8 +1,8 @@ -# aws-smithy-retries +# aws-smithy-runtime-api **This crate is UNSTABLE! All internal and external interfaces are subject to change without notice.** -Smithy runtime types. +Lightweight crate with traits and types necessary to configure runtime logic in the `aws-smithy-runtime` crate. This crate is part of the [AWS SDK for Rust](https://awslabs.github.io/aws-sdk-rust/) and the [smithy-rs](https://github.com/awslabs/smithy-rs) code generator. In most cases, it should not be used directly. diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs index 2a1b6c8d239..a8ab993960e 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs @@ -7,6 +7,7 @@ use crate::client::auth::AuthSchemeId; use crate::client::orchestrator::Future; use aws_smithy_types::config_bag::{ConfigBag, Storable, StoreAppend, StoreReplace}; use std::any::Any; +use std::fmt; use std::fmt::Debug; use std::sync::Arc; use std::time::SystemTime; @@ -96,21 +97,27 @@ impl IdentityResolvers { } } -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct Identity { data: Arc, + #[allow(clippy::type_complexity)] + data_debug: Arc) -> &dyn Debug) + Send + Sync>, expiration: Option, } impl Identity { - pub fn new(data: impl Any + Send + Sync, expiration: Option) -> Self { + pub fn new(data: T, expiration: Option) -> Self + where + T: Any + Debug + Send + Sync, + { Self { data: Arc::new(data), + data_debug: Arc::new(|d| d.downcast_ref::().expect("type-checked") as _), expiration, } } - pub fn data(&self) -> Option<&T> { + pub fn data(&self) -> Option<&T> { self.data.downcast_ref() } @@ -119,6 +126,15 @@ impl Identity { } } +impl fmt::Debug for Identity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Identity") + .field("data", (self.data_debug)(&self.data)) + .field("expiration", &self.expiration) + .finish() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs index dd918faba02..5d93c645660 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs @@ -673,6 +673,11 @@ macro_rules! interceptor_impl_fn { ctx: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!(concat!( + "running `", + stringify!($interceptor), + "` interceptors" + )); let mut result: Result<(), BoxError> = Ok(()); let mut ctx = ctx.into(); for interceptor in self.interceptors() { @@ -774,6 +779,7 @@ impl Interceptors { ctx: &InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `client_read_before_execution` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let ctx: BeforeSerializationInterceptorContextRef<'_> = ctx.into(); for interceptor in self.client_interceptors() { @@ -794,6 +800,7 @@ impl Interceptors { ctx: &InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `operation_read_before_execution` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let ctx: BeforeSerializationInterceptorContextRef<'_> = ctx.into(); for interceptor in self.operation_interceptors() { @@ -829,6 +836,7 @@ impl Interceptors { ctx: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `modify_before_attempt_completion` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let mut ctx: FinalizerInterceptorContextMut<'_> = ctx.into(); for interceptor in self.interceptors() { @@ -850,6 +858,7 @@ impl Interceptors { ctx: &InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `read_after_attempt` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let ctx: FinalizerInterceptorContextRef<'_> = ctx.into(); for interceptor in self.interceptors() { @@ -870,6 +879,7 @@ impl Interceptors { ctx: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `modify_before_completion` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let mut ctx: FinalizerInterceptorContextMut<'_> = ctx.into(); for interceptor in self.interceptors() { @@ -890,6 +900,7 @@ impl Interceptors { ctx: &InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), InterceptorError> { + tracing::trace!("running `read_after_execution` interceptors"); let mut result: Result<(), BoxError> = Ok(()); let ctx: FinalizerInterceptorContextRef<'_> = ctx.into(); for interceptor in self.interceptors() { diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs index 15a29c3ba57..d0a8e18b59c 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs @@ -70,6 +70,7 @@ impl RuntimePlugins { cfg: &mut ConfigBag, interceptors: &mut InterceptorRegistrar, ) -> Result<(), BoxError> { + tracing::trace!("applying client runtime plugins"); for plugin in self.client_plugins.iter() { if let Some(layer) = plugin.config() { cfg.push_shared_layer(layer); @@ -85,6 +86,7 @@ impl RuntimePlugins { cfg: &mut ConfigBag, interceptors: &mut InterceptorRegistrar, ) -> Result<(), BoxError> { + tracing::trace!("applying operation runtime plugins"); for plugin in self.operation_plugins.iter() { if let Some(layer) = plugin.config() { cfg.push_shared_layer(layer); diff --git a/rust-runtime/aws-smithy-runtime/Cargo.toml b/rust-runtime/aws-smithy-runtime/Cargo.toml index d63a748b5cf..b481d7ff99b 100644 --- a/rust-runtime/aws-smithy-runtime/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/awslabs/smithy-rs" [features] http-auth = ["aws-smithy-runtime-api/http-auth"] -test-util = ["dep:aws-smithy-protocol-test"] +test-util = ["dep:aws-smithy-protocol-test", "dep:tracing-subscriber"] [dependencies] aws-smithy-async = { path = "../aws-smithy-async" } @@ -21,14 +21,15 @@ aws-smithy-protocol-test = { path = "../aws-smithy-protocol-test", optional = tr aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api" } aws-smithy-types = { path = "../aws-smithy-types" } bytes = "1" +fastrand = "1.4" http = "0.2.8" http-body = "0.4.5" +once_cell = "1.18.0" pin-project-lite = "0.2.7" pin-utils = "0.1.0" tokio = { version = "1.25", features = [] } tracing = "0.1.37" -fastrand = "1.4" -once_cell = "1.18.0" +tracing-subscriber = { version = "0.3.16", optional = true, features = ["fmt", "json"] } [dev-dependencies] approx = "0.5.1" @@ -36,7 +37,7 @@ aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio", "test aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api", features = ["test-util"] } aws-smithy-types = { path = "../aws-smithy-types", features = ["test-util"] } tokio = { version = "1.25", features = ["macros", "rt", "test-util"] } -tracing-subscriber = { version = "0.3.15", features = ["env-filter"] } +tracing-subscriber = { version = "0.3.16", features = ["env-filter"] } tracing-test = "0.2.1" [package.metadata.docs.rs] diff --git a/rust-runtime/aws-smithy-runtime/README.md b/rust-runtime/aws-smithy-runtime/README.md index ba6dbc29ae1..f283643f8c7 100644 --- a/rust-runtime/aws-smithy-runtime/README.md +++ b/rust-runtime/aws-smithy-runtime/README.md @@ -1,8 +1,8 @@ -# aws-smithy-orchestrator +# aws-smithy-runtime **This crate is UNSTABLE! All internal and external interfaces are subject to change without notice.** -Code which enables users to access a smithy service. Handles the configuration and construction of requests, as well as responses. +Runtime support logic and types for smithy-rs generated code. This crate is part of the [AWS SDK for Rust](https://awslabs.github.io/aws-sdk-rust/) and the [smithy-rs](https://github.com/awslabs/smithy-rs) code generator. In most cases, it should not be used directly. diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index de6f9dceaad..552c78b04b9 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -56,19 +56,27 @@ macro_rules! halt_on_err { macro_rules! continue_on_err { ([$ctx:ident] => $expr:expr) => { if let Err(err) = $expr { - debug!("encountered orchestrator error; continuing"); + debug!(err = ?err, "encountered orchestrator error; continuing"); $ctx.fail(err.into()); } }; } pub async fn invoke( + service_name: &str, + operation_name: &str, input: Input, runtime_plugins: &RuntimePlugins, ) -> Result> { - invoke_with_stop_point(input, runtime_plugins, StopPoint::None) - .await? - .finalize() + invoke_with_stop_point( + service_name, + operation_name, + input, + runtime_plugins, + StopPoint::None, + ) + .await? + .finalize() } /// Allows for returning early at different points during orchestration. @@ -82,32 +90,38 @@ pub enum StopPoint { BeforeTransmit, } -#[tracing::instrument(skip_all, name = "invoke")] pub async fn invoke_with_stop_point( + service_name: &str, + operation_name: &str, input: Input, runtime_plugins: &RuntimePlugins, stop_point: StopPoint, ) -> Result> { - let mut cfg = ConfigBag::base(); - let cfg = &mut cfg; + async move { + let mut cfg = ConfigBag::base(); + let cfg = &mut cfg; - let mut interceptors = Interceptors::new(); - let mut ctx = InterceptorContext::new(input); + let mut interceptors = Interceptors::new(); + let mut ctx = InterceptorContext::new(input); - if let Err(err) = apply_configuration(&mut ctx, cfg, &mut interceptors, runtime_plugins) { - return Err(SdkError::construction_failure(err)); - } - let operation_timeout_config = cfg.maybe_timeout_config(TimeoutKind::Operation); - async { - // If running the pre-execution interceptors failed, then we skip running the op and run the - // final interceptors instead. - if !ctx.is_failed() { - try_op(&mut ctx, cfg, &interceptors, stop_point).await; + if let Err(err) = apply_configuration(&mut ctx, cfg, &mut interceptors, runtime_plugins) { + return Err(SdkError::construction_failure(err)); + } + let operation_timeout_config = cfg.maybe_timeout_config(TimeoutKind::Operation); + trace!(operation_timeout_config = ?operation_timeout_config); + async { + // If running the pre-execution interceptors failed, then we skip running the op and run the + // final interceptors instead. + if !ctx.is_failed() { + try_op(&mut ctx, cfg, &interceptors, stop_point).await; + } + finally_op(&mut ctx, cfg, &interceptors).await; + Ok(ctx) } - finally_op(&mut ctx, cfg, &interceptors).await; - Ok(ctx) + .maybe_timeout_with_config(operation_timeout_config) + .await } - .maybe_timeout_with_config(operation_timeout_config) + .instrument(debug_span!("invoke", service = %service_name, operation = %operation_name)) .await } @@ -123,6 +137,7 @@ fn apply_configuration( ) -> Result<(), BoxError> { runtime_plugins.apply_client_configuration(cfg, interceptors.client_interceptors_mut())?; continue_on_err!([ctx] => interceptors.client_read_before_execution(ctx, cfg)); + runtime_plugins .apply_operation_configuration(cfg, interceptors.operation_interceptors_mut())?; continue_on_err!([ctx] => interceptors.operation_read_before_execution(ctx, cfg)); @@ -144,6 +159,7 @@ async fn try_op( // Serialization ctx.enter_serialization_phase(); { + let _span = debug_span!("serialization").entered(); let request_serializer = cfg.request_serializer(); let input = ctx.take_input().expect("input set at this point"); let request = halt_on_err!([ctx] => request_serializer.serialize_input(input, cfg).map_err(OrchestratorError::other)); @@ -152,6 +168,7 @@ async fn try_op( // Load the request body into memory if configured to do so if let LoadedRequestBody::Requested = cfg.loaded_request_body() { + debug!("loading request body into memory"); let mut body = SdkBody::taken(); mem::swap(&mut body, ctx.request_mut().expect("set above").body_mut()); let loaded_body = halt_on_err!([ctx] => ByteStream::new(body).collect().await).into_bytes(); @@ -196,9 +213,9 @@ async fn try_op( ctx.save_checkpoint(); let mut retry_delay = None; for i in 1u32.. { - debug!("beginning attempt #{i}"); // Break from the loop if we can't rewind the request's state. This will always succeed the // first time, but will fail on subsequent iterations if the request body wasn't retryable. + trace!("checking if context can be rewound for attempt #{i}"); if let RewindResult::Impossible = ctx.rewind(cfg) { debug!("request cannot be retried since the request body cannot be cloned"); break; @@ -206,13 +223,15 @@ async fn try_op( // Track which attempt we're currently on. cfg.interceptor_state() .store_put::(i.into()); + // Backoff time should not be included in the attempt timeout + if let Some((delay, sleep)) = retry_delay.take() { + debug!("delaying for {delay:?}"); + sleep.await; + } let attempt_timeout_config = cfg.maybe_timeout_config(TimeoutKind::OperationAttempt); + trace!(attempt_timeout_config = ?attempt_timeout_config); let maybe_timeout = async { - // We must await this here or else timeouts won't work as expected - if let Some(delay) = retry_delay.take() { - delay.await; - } - + debug!("beginning attempt #{i}"); try_attempt(ctx, cfg, interceptors, stop_point).await; finally_attempt(ctx, cfg, interceptors).await; Result::<_, SdkError>::Ok(()) @@ -246,7 +265,7 @@ async fn try_op( let sleep_impl = halt_on_err!([ctx] => cfg.sleep_impl().ok_or(OrchestratorError::other( "the retry strategy requested a delay before sending the retry request, but no 'async sleep' implementation was set" ))); - retry_delay = Some(sleep_impl.sleep(delay)); + retry_delay = Some((delay, sleep_impl.sleep(delay))); continue; } } @@ -261,7 +280,9 @@ async fn try_attempt( stop_point: StopPoint, ) { halt_on_err!([ctx] => interceptors.read_before_attempt(ctx, cfg)); + halt_on_err!([ctx] => orchestrate_endpoint(ctx, cfg).await.map_err(OrchestratorError::other)); + halt_on_err!([ctx] => interceptors.modify_before_signing(ctx, cfg)); halt_on_err!([ctx] => interceptors.read_before_signing(ctx, cfg)); @@ -273,14 +294,16 @@ async fn try_attempt( // Return early if a stop point is set for before transmit if let StopPoint::BeforeTransmit = stop_point { + debug!("ending orchestration early because the stop point is `BeforeTransmit`"); return; } // The connection consumes the request but we need to keep a copy of it // within the interceptor context, so we clone it here. ctx.enter_transmit_phase(); - let call_result = halt_on_err!([ctx] => { + let response = halt_on_err!([ctx] => { let request = ctx.take_request().expect("set during serialization"); + trace!(request = ?request, "transmitting request"); cfg.connector().call(request).await.map_err(|err| { match err.downcast() { Ok(connector_error) => OrchestratorError::connector(*connector_error), @@ -288,8 +311,8 @@ async fn try_attempt( } }) }); - trace!(response = ?call_result, "received response from service"); - ctx.set_response(call_result); + trace!(response = ?response, "received response from service"); + ctx.set_response(response); ctx.enter_before_deserialization_phase(); halt_on_err!([ctx] => interceptors.read_after_transmit(ctx, cfg)); @@ -300,16 +323,25 @@ async fn try_attempt( let output_or_error = async { let response = ctx.response_mut().expect("set during transmit"); let response_deserializer = cfg.response_deserializer(); - match response_deserializer.deserialize_streaming(response) { + let maybe_deserialized = { + let _span = debug_span!("deserialize_streaming").entered(); + response_deserializer.deserialize_streaming(response) + }; + match maybe_deserialized { Some(output_or_error) => output_or_error, None => read_body(response) .instrument(debug_span!("read_body")) .await .map_err(OrchestratorError::response) - .and_then(|_| response_deserializer.deserialize_nonstreaming(response)), + .and_then(|_| { + let _span = debug_span!("deserialize_nonstreaming").entered(); + response_deserializer.deserialize_nonstreaming(response) + }), } } + .instrument(debug_span!("deserialization")) .await; + trace!(output_or_error = ?output_or_error); ctx.set_output_or_error(output_or_error); ctx.enter_after_deserialization_phase(); @@ -336,20 +368,21 @@ async fn finally_op( continue_on_err!([ctx] => interceptors.read_after_execution(ctx, cfg)); } -#[cfg(all(test, feature = "test-util", feature = "anonymous-auth"))] +#[cfg(all(test, feature = "test-util"))] mod tests { use super::*; - use crate::client::auth::no_auth::NoAuthRuntimePlugin; + use crate::client::auth::no_auth::{NoAuthRuntimePlugin, NO_AUTH_SCHEME_ID}; use crate::client::orchestrator::endpoints::{ StaticUriEndpointResolver, StaticUriEndpointResolverParams, }; use crate::client::retries::strategy::NeverRetryStrategy; use crate::client::test_util::{ - connector::OkConnector, deserializer::CannedResponseDeserializer, - serializer::CannedRequestSerializer, + deserializer::CannedResponseDeserializer, serializer::CannedRequestSerializer, }; use ::http::{Request, Response, StatusCode}; - use aws_smithy_runtime_api::client::connectors::Connector; + use aws_smithy_runtime_api::client::auth::option_resolver::StaticAuthOptionResolver; + use aws_smithy_runtime_api::client::auth::{AuthOptionResolverParams, DynAuthOptionResolver}; + use aws_smithy_runtime_api::client::connectors::{Connector, DynConnector}; use aws_smithy_runtime_api::client::interceptors::context::{ AfterDeserializationInterceptorContextRef, BeforeDeserializationInterceptorContextMut, BeforeDeserializationInterceptorContextRef, BeforeSerializationInterceptorContextMut, @@ -361,7 +394,8 @@ mod tests { Interceptor, InterceptorRegistrar, SharedInterceptor, }; use aws_smithy_runtime_api::client::orchestrator::{ - DynConnector, DynEndpointResolver, DynResponseDeserializer, SharedRequestSerializer, + BoxFuture, DynEndpointResolver, DynResponseDeserializer, Future, HttpRequest, + SharedRequestSerializer, }; use aws_smithy_runtime_api::client::retries::DynRetryStrategy; use aws_smithy_runtime_api::client::runtime_plugin::{RuntimePlugin, RuntimePlugins}; @@ -390,17 +424,17 @@ mod tests { } #[derive(Debug, Default)] - pub struct OkConnector {} + struct OkConnector {} impl OkConnector { - pub fn new() -> Self { + fn new() -> Self { Self::default() } } impl Connector for OkConnector { fn call(&self, _request: HttpRequest) -> BoxFuture { - Box::pin(Future::ready(Ok(http::Response::builder() + Box::pin(Future::ready(Ok(::http::Response::builder() .status(200) .body(SdkBody::empty()) .expect("OK response is valid")))) @@ -423,6 +457,10 @@ mod tests { )); cfg.set_endpoint_resolver_params(StaticUriEndpointResolverParams::new().into()); cfg.set_connector(DynConnector::new(OkConnector::new())); + cfg.set_auth_option_resolver_params(AuthOptionResolverParams::new("idontcare")); + cfg.set_auth_option_resolver(DynAuthOptionResolver::new( + StaticAuthOptionResolver::new(vec![NO_AUTH_SCHEME_ID]), + )); Some(cfg.freeze()) } @@ -482,7 +520,7 @@ mod tests { .with_operation_plugin(TestOperationRuntimePlugin) .with_operation_plugin(NoAuthRuntimePlugin::new()) .with_operation_plugin(FailingInterceptorsOperationRuntimePlugin); - let actual = invoke(input, &runtime_plugins) + let actual = invoke("test", "test", input, &runtime_plugins) .await .expect_err("should error"); let actual = format!("{:?}", actual); @@ -744,9 +782,9 @@ mod tests { let input = TypeErasedBox::new(Box::new(())); let runtime_plugins = RuntimePlugins::new() .with_operation_plugin(TestOperationRuntimePlugin) - .with_operation_plugin(AnonymousAuthRuntimePlugin::new()) + .with_operation_plugin(NoAuthRuntimePlugin::new()) .with_operation_plugin(InterceptorsTestOperationRuntimePlugin); - let actual = invoke(input, &runtime_plugins) + let actual = invoke("test", "test", input, &runtime_plugins) .await .expect_err("should error"); let actual = format!("{:?}", actual); @@ -992,11 +1030,13 @@ mod tests { let runtime_plugins = || { RuntimePlugins::new() .with_operation_plugin(TestOperationRuntimePlugin) - .with_operation_plugin(AnonymousAuthRuntimePlugin::new()) + .with_operation_plugin(NoAuthRuntimePlugin::new()) }; // StopPoint::None should result in a response getting set since orchestration doesn't stop let context = invoke_with_stop_point( + "test", + "test", TypedBox::new(()).erase(), &runtime_plugins(), StopPoint::None, @@ -1007,6 +1047,8 @@ mod tests { // StopPoint::BeforeTransmit will exit right before sending the request, so there should be no response let context = invoke_with_stop_point( + "test", + "test", TypedBox::new(()).erase(), &runtime_plugins(), StopPoint::BeforeTransmit, @@ -1079,7 +1121,7 @@ mod tests { let runtime_plugins = || { RuntimePlugins::new() .with_operation_plugin(TestOperationRuntimePlugin) - .with_operation_plugin(AnonymousAuthRuntimePlugin::new()) + .with_operation_plugin(NoAuthRuntimePlugin::new()) .with_operation_plugin(TestInterceptorRuntimePlugin { interceptor: interceptor.clone(), }) @@ -1087,6 +1129,8 @@ mod tests { // StopPoint::BeforeTransmit will exit right before sending the request, so there should be no response let context = invoke_with_stop_point( + "test", + "test", TypedBox::new(()).erase(), &runtime_plugins(), StopPoint::BeforeTransmit, diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs index 92191782bce..ce645708709 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs @@ -16,6 +16,7 @@ use aws_smithy_types::Document; use std::borrow::Cow; use std::error::Error as StdError; use std::fmt; +use tracing::trace; #[derive(Debug)] enum AuthOrchestrationError { @@ -71,7 +72,7 @@ pub(super) async fn orchestrate_auth( let auth_options = cfg.auth_option_resolver().resolve_auth_options(params)?; let identity_resolvers = cfg.identity_resolvers(); - tracing::trace!( + trace!( auth_option_resolver_params = ?params, auth_options = ?auth_options, identity_resolvers = ?identity_resolvers, @@ -82,13 +83,24 @@ pub(super) async fn orchestrate_auth( if let Some(auth_scheme) = cfg.http_auth_schemes().scheme(scheme_id) { if let Some(identity_resolver) = auth_scheme.identity_resolver(&identity_resolvers) { let request_signer = auth_scheme.request_signer(); + trace!( + auth_scheme = ?auth_scheme, + identity_resolver = ?identity_resolver, + request_signer = ?request_signer, + "resolved auth scheme, identity resolver, and signing implementation" + ); + let endpoint = cfg .load::() .expect("endpoint added to config bag by endpoint orchestrator"); let auth_scheme_endpoint_config = extract_endpoint_auth_scheme_config(endpoint, scheme_id)?; + trace!(auth_scheme_endpoint_config = ?auth_scheme_endpoint_config, "extracted auth scheme endpoint config"); let identity = identity_resolver.resolve_identity(cfg).await?; + trace!(identity = ?identity, "resolved identity"); + + trace!("signing request"); let request = ctx.request_mut().expect("set during serialization"); request_signer.sign_request( request, diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs index 12db9cb6066..394cde080b1 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs @@ -20,6 +20,7 @@ use http::header::HeaderName; use http::{HeaderValue, Uri}; use std::fmt::Debug; use std::str::FromStr; +use tracing::trace; #[derive(Debug, Clone)] pub struct StaticUriEndpointResolver { @@ -104,6 +105,8 @@ pub(super) async fn orchestrate_endpoint( ctx: &mut InterceptorContext, cfg: &mut ConfigBag, ) -> Result<(), BoxError> { + trace!("orchestrating endpoint resolution"); + let params = cfg.endpoint_resolver_params(); let endpoint_prefix = cfg.load::(); let request = ctx.request_mut().expect("set during serialization"); diff --git a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs index 307297e0c5a..ecb29234f4d 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/retries/strategy/standard.rs @@ -241,7 +241,7 @@ impl RetryStrategy for StandardRetryStrategy { debug!( "attempt #{request_attempts} failed with {:?}; retrying after {:?}", retry_reason.expect("the match statement above ensures this is not None"), - backoff + backoff, ); Ok(ShouldAttempt::YesAfterDelay(backoff)) diff --git a/rust-runtime/aws-smithy-runtime/src/lib.rs b/rust-runtime/aws-smithy-runtime/src/lib.rs index 2c2eb6a9f8e..cd4364a9b35 100644 --- a/rust-runtime/aws-smithy-runtime/src/lib.rs +++ b/rust-runtime/aws-smithy-runtime/src/lib.rs @@ -3,13 +3,26 @@ * SPDX-License-Identifier: Apache-2.0 */ +//! Runtime support logic and types for smithy-rs generated code. +//! +//! # Crate Features +//! +//! - `http-auth`: Enables auth scheme and identity resolver implementations for HTTP API Key, +//! Basic Auth, Bearer Token, and Digest Auth. +//! - `test-util`: Enables utilities for unit tests. DO NOT ENABLE IN PRODUCTION. + #![warn( // missing_docs, - // rustdoc::missing_crate_level_docs, + rustdoc::missing_crate_level_docs, unreachable_pub, rust_2018_idioms )] +/// Runtime support logic for generated clients. pub mod client; pub mod static_partition_map; + +/// General testing utilities. +#[cfg(feature = "test-util")] +pub mod test_util; diff --git a/rust-runtime/aws-smithy-runtime/src/test_util.rs b/rust-runtime/aws-smithy-runtime/src/test_util.rs new file mode 100644 index 00000000000..b3ec5e5dfb3 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime/src/test_util.rs @@ -0,0 +1,7 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +/// Utility for capturing and displaying logs during a unit test. +pub mod capture_test_logs; diff --git a/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs b/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs new file mode 100644 index 00000000000..a25c97c42f8 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs @@ -0,0 +1,90 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use std::env; +use std::io::Write; +use std::sync::{Arc, Mutex}; +use tracing::subscriber::DefaultGuard; +use tracing::Level; +use tracing_subscriber::fmt::TestWriter; + +struct Tee { + buf: Arc>>, + quiet: bool, + inner: W, +} + +/// A guard that resets log capturing upon being dropped. +#[derive(Debug)] +pub struct LogCaptureGuard(DefaultGuard); + +/// Capture logs from this test. +/// +/// The logs will be captured until the `DefaultGuard` is dropped. +/// +/// *Why use this instead of traced_test?* +/// This captures _all_ logs, not just logs produced by the current crate. +#[must_use] // log capturing ceases the instant the `DefaultGuard` is dropped +pub fn capture_test_logs() -> (LogCaptureGuard, Rx) { + // it may be helpful to upstream this at some point + let (mut writer, rx) = Tee::stdout(); + if env::var("VERBOSE_TEST_LOGS").is_ok() { + eprintln!("Enabled verbose test logging."); + writer.loud(); + } else { + eprintln!("To see full logs from this test set VERBOSE_TEST_LOGS=true"); + } + let subscriber = tracing_subscriber::fmt() + .with_max_level(Level::TRACE) + .with_writer(Mutex::new(writer)) + .finish(); + let guard = tracing::subscriber::set_default(subscriber); + (LogCaptureGuard(guard), rx) +} + +pub struct Rx(Arc>>); +impl Rx { + pub fn contents(&self) -> String { + String::from_utf8(self.0.lock().unwrap().clone()).unwrap() + } +} + +impl Tee { + fn stdout() -> (Self, Rx) { + let buf: Arc>> = Default::default(); + ( + Tee { + buf: buf.clone(), + quiet: true, + inner: TestWriter::new(), + }, + Rx(buf), + ) + } +} + +impl Tee { + fn loud(&mut self) { + self.quiet = false; + } +} + +impl Write for Tee +where + W: Write, +{ + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.buf.lock().unwrap().extend_from_slice(buf); + if !self.quiet { + self.inner.write(buf) + } else { + Ok(buf.len()) + } + } + + fn flush(&mut self) -> std::io::Result<()> { + self.inner.flush() + } +} diff --git a/tools/ci-scripts/check-aws-sdk-orchestrator-impl b/tools/ci-scripts/check-aws-sdk-orchestrator-impl index 413082fa7cd..a434b07761b 100755 --- a/tools/ci-scripts/check-aws-sdk-orchestrator-impl +++ b/tools/ci-scripts/check-aws-sdk-orchestrator-impl @@ -18,11 +18,6 @@ services_that_dont_compile=(\ "timestreamwrite", ) -# TODO(enableNewSmithyRuntimeLaunch): Move these into `services_that_pass_tests` as more progress is made -services_that_compile=(\ - "s3"\ -) - services_that_pass_tests=(\ "aws-config"\ "config"\ @@ -45,15 +40,6 @@ services_that_pass_tests=(\ ./gradlew aws:sdk:assemble -Psmithy.runtime.mode=orchestrator cd aws/sdk/build/aws-sdk/sdk -for service in "${services_that_compile[@]}"; do - pushd "${service}" - echo -e "${C_YELLOW}# Running 'cargo check --all-features --all-targets' on '${service}'${C_RESET}" - RUSTFLAGS="${RUSTFLAGS:-} --cfg aws_sdk_orchestrator_mode" cargo check --all-features --all-targets - echo -e "${C_YELLOW}# Running 'cargo clippy --all-features' on '${service}'${C_RESET}" - RUSTFLAGS="${RUSTFLAGS:-} --cfg aws_sdk_orchestrator_mode" cargo clippy --all-features - popd -done - for service in "${services_that_pass_tests[@]}"; do pushd "${service}" echo -e "${C_YELLOW}# Running 'cargo test --all-features' on '${service}'${C_RESET}" From d75382789a3d58e6c832d59dd0cc7cce90477a58 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 30 Jun 2023 19:40:27 -0500 Subject: [PATCH 5/6] Implement the remaining part of supporting operation level configuration (#2814) ## Motivation and Context Implements the actual meat of `config_override` introduced [as a skeleton in the past](https://github.com/awslabs/smithy-rs/pull/2589). ## Description This PR enables operation-level config via `config_override` on a customizable operation (see [config-override.rs](https://github.com/awslabs/smithy-rs/blob/8105dd46b43854e7909aed82c85223414bc85df5/aws/sdk/integration-tests/s3/tests/config-override.rs) integration tests for example code snippets). The way it's implemented is through `ConfigOverrideRuntimePlugin`. The plugin holds onto two things: a `Builder` passed to `config_override` and a `FrozenLayer` derived from a service config (the latter is primarily for retrieving default values understood by a service config). The plugin then implements the `RuntimePlugin` trait to generate its own `FrozenLayer` that contains operation-level orchestrator components. That `FrozenLayer` will then be added to a config bag later in the orchestrator execution in a way that it takes precedence over the client-level configuration (see [register_default_runtime_plugins](https://github.com/awslabs/smithy-rs/blob/8105dd46b43854e7909aed82c85223414bc85df5/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt#L65-L71)). A couple of things to note: - The size of `ConfigOverrideRuntimePlugin::config` code-generated is getting large. Maybe each customization defines a helper function instead of inlining logic directly in `config` and we let the `config` method call those generated helpers. - The PR does not handle a case where `retry_partition` within `config_override` since it's currently `#[doc(hidden)]`, e.g. ``` client .some_operation() .customize() .await .unwrap() .config_override(Config::builder().retry_partition(/* ... */)) ... ``` ## Testing - Added tests in Kotlin [ConfigOverrideRuntimePluginGeneratorTest.kt](https://github.com/awslabs/smithy-rs/pull/2814/files#diff-04a76a43c2adb3a2ee37443157c68ec6dad77fab2484722b370a7ba14cf02086) and [CredentialCacheConfigTest.kt](https://github.com/awslabs/smithy-rs/pull/2814/files#diff-32246072688cd11391fa10cd9cb38a80ed88b587e95037225dbe9f1a482ebc5d). ~~These tests are minimal in that they only check if the appropriate orchestrator components are inserted into a config override layer when a user calls a certain builder method as part of `config_override`.~~ - Added integration tests [config-override.rs](https://github.com/awslabs/smithy-rs/pull/2814/files#diff-6fd7a1e70b1c3fa3e9c747925f3fc7a6ce0f7b801bd6bc46c54eec44150f5158) ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito --- .../smithy/rustsdk/AwsPresigningDecorator.kt | 3 +- .../amazon/smithy/rustsdk/CredentialCaches.kt | 30 +++ .../rustsdk/CredentialCacheConfigTest.kt | 188 +++++++++++++ .../amazon/smithy/rustsdk/TestUtil.kt | 7 + .../s3/tests/config-override.rs | 121 +++++++++ .../HttpConnectorConfigDecorator.kt | 39 +++ .../InterceptorConfigCustomization.kt | 2 +- .../ResiliencyConfigCustomization.kt | 26 +- .../endpoint/EndpointConfigCustomization.kt | 18 ++ .../ConfigOverrideRuntimePluginGenerator.kt | 82 ++++++ .../smithy/generators/OperationGenerator.kt | 6 +- .../smithy/generators/PaginatorGenerator.kt | 3 +- .../smithy/generators/ServiceGenerator.kt | 3 +- .../client/FluentClientGenerator.kt | 3 +- .../config/ServiceConfigGenerator.kt | 56 ++-- ...onfigOverrideRuntimePluginGeneratorTest.kt | 247 ++++++++++++++++++ .../generators/PaginatorGeneratorTest.kt | 13 +- .../codegen/core/rustlang/CargoDependency.kt | 2 + .../smithy/rust/codegen/core/testutil/Rust.kt | 2 + .../aws-smithy-types/src/config_bag.rs | 7 + 20 files changed, 800 insertions(+), 58 deletions(-) create mode 100644 aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt create mode 100644 aws/sdk/integration-tests/s3/tests/config-override.rs create mode 100644 codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGenerator.kt create mode 100644 codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt index 5ffd15a6a8a..ef59287b35d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsPresigningDecorator.kt @@ -322,7 +322,8 @@ class AwsPresignedFluentBuilderMethod( let runtime_plugins = #{Operation}::operation_runtime_plugins( self.handle.runtime_plugins.clone(), - self.config_override + &self.handle.conf, + self.config_override, ) .with_client_plugin(#{SigV4PresigningRuntimePlugin}::new(presigning_config, #{payload_override})) #{alternate_presigning_serializer_registration}; diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt index 053e7ec7e6a..b31c5329ea9 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt @@ -191,6 +191,36 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom } } + is ServiceConfig.OperationConfigOverride -> { + rustTemplate( + """ + match ( + layer + .load::<#{CredentialsCache}>() + .cloned(), + layer + .load::<#{SharedCredentialsProvider}>() + .cloned(), + ) { + (#{None}, #{None}) => {} + (#{None}, _) => { + panic!("also specify `.credentials_cache` when overriding credentials provider for the operation"); + } + (_, #{None}) => { + panic!("also specify `.credentials_provider` when overriding credentials cache for the operation"); + } + ( + #{Some}(credentials_cache), + #{Some}(credentials_provider), + ) => { + layer.store_put(credentials_cache.create_cache(credentials_provider)); + } + } + """, + *codegenScope, + ) + } + else -> emptySection } } diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt new file mode 100644 index 00000000000..c598c1ab0d4 --- /dev/null +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt @@ -0,0 +1,188 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rustsdk + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.core.rustlang.Attribute +import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.testModule +import software.amazon.smithy.rust.codegen.core.testutil.tokioTest +import software.amazon.smithy.rust.codegen.core.testutil.unitTest + +internal class CredentialCacheConfigTest { + private val model = """ + namespace com.example + use aws.protocols#awsJson1_0 + use aws.api#service + use smithy.rules#endpointRuleSet + + @service(sdkId: "Some Value") + @awsJson1_0 + @endpointRuleSet({ + "version": "1.0", + "rules": [{ + "type": "endpoint", + "conditions": [{"fn": "isSet", "argv": [{"ref": "Region"}]}], + "endpoint": { "url": "https://example.com" } + }], + "parameters": { + "Region": { "required": false, "type": "String", "builtIn": "AWS::Region" }, + } + }) + service HelloService { + operations: [SayHello], + version: "1" + } + + @optionalAuth + operation SayHello { input: TestInput } + structure TestInput { + foo: String, + } + """.asSmithyModel() + + @Test + fun `config override for credentials`() { + awsSdkIntegrationTest(model, defaultToOrchestrator = true) { clientCodegenContext, rustCrate -> + val runtimeConfig = clientCodegenContext.runtimeConfig + val codegenScope = arrayOf( + *RuntimeType.preludeScope, + "Credentials" to AwsRuntimeType.awsCredentialTypesTestUtil(runtimeConfig) + .resolve("Credentials"), + "CredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("cache::CredentialsCache"), + "ProvideCachedCredentials" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("cache::ProvideCachedCredentials"), + "RuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::runtime_plugin::RuntimePlugin"), + "SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("cache::SharedCredentialsCache"), + "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("provider::SharedCredentialsProvider"), + ) + rustCrate.testModule { + unitTest( + "test_overriding_only_credentials_provider_should_panic", + additionalAttributes = listOf(Attribute.shouldPanic("also specify `.credentials_cache` when overriding credentials provider for the operation")), + ) { + rustTemplate( + """ + use #{RuntimePlugin}; + + let client_config = crate::config::Config::builder().build(); + let config_override = + crate::config::Config::builder().credentials_provider(#{Credentials}::for_tests()); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config.config().unwrap(), + config_override, + }; + + // this should cause `panic!` + let _ = sut.config().unwrap(); + """, + *codegenScope, + ) + } + + unitTest( + "test_overriding_only_credentials_cache_should_panic", + additionalAttributes = listOf(Attribute.shouldPanic("also specify `.credentials_provider` when overriding credentials cache for the operation")), + ) { + rustTemplate( + """ + use #{RuntimePlugin}; + + let client_config = crate::config::Config::builder().build(); + let config_override = crate::config::Config::builder() + .credentials_cache(#{CredentialsCache}::no_caching()); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config.config().unwrap(), + config_override, + }; + + // this should cause `panic!` + let _ = sut.config().unwrap(); + """, + *codegenScope, + ) + } + + tokioTest("test_overriding_cache_and_provider_leads_to_shared_credentials_cache_in_layer") { + rustTemplate( + """ + use #{ProvideCachedCredentials}; + use #{RuntimePlugin}; + + let client_config = crate::config::Config::builder() + .credentials_provider(#{Credentials}::for_tests()) + .build(); + let client_config_layer = client_config.config().unwrap(); + + // make sure test credentials are set in the client config level + assert_eq!(#{Credentials}::for_tests(), + client_config_layer + .load::<#{SharedCredentialsCache}>() + .unwrap() + .provide_cached_credentials() + .await + .unwrap() + ); + + let credentials = #{Credentials}::new( + "test", + "test", + #{None}, + #{None}, + "test", + ); + let config_override = crate::config::Config::builder() + .credentials_cache(#{CredentialsCache}::lazy()) + .credentials_provider(credentials.clone()); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config_layer, + config_override, + }; + let sut_layer = sut.config().unwrap(); + + // make sure `.provide_cached_credentials` returns credentials set through `config_override` + assert_eq!(credentials, + sut_layer + .load::<#{SharedCredentialsCache}>() + .unwrap() + .provide_cached_credentials() + .await + .unwrap() + ); + """, + *codegenScope, + ) + } + + unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") { + rustTemplate( + """ + use #{RuntimePlugin}; + + let client_config = crate::config::Config::builder().build(); + let config_override = crate::config::Config::builder(); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config.config().unwrap(), + config_override, + }; + let sut_layer = sut.config().unwrap(); + assert!(sut_layer + .load::<#{SharedCredentialsCache}>() + .is_none()); + """, + *codegenScope, + ) + } + } + } + } +} diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt index fc40eec7cd3..0dabd0033e6 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/TestUtil.kt @@ -7,6 +7,7 @@ package software.amazon.smithy.rustsdk import software.amazon.smithy.model.Model import software.amazon.smithy.model.node.ObjectNode +import software.amazon.smithy.model.node.StringNode import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustSettings import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest @@ -17,6 +18,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams import software.amazon.smithy.rust.codegen.core.testutil.TestRuntimeConfig import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.util.letIf import java.io.File // In aws-sdk-codegen, the working dir when gradle runs tests is actually `./aws`. So, to find the smithy runtime, we need @@ -35,8 +37,10 @@ fun awsTestCodegenContext(model: Model? = null, settings: ClientRustSettings? = settings = settings ?: testClientRustSettings(runtimeConfig = AwsTestRuntimeConfig), ) +// TODO(enableNewSmithyRuntimeCleanup): Remove defaultToOrchestrator once the runtime switches to the orchestrator fun awsSdkIntegrationTest( model: Model, + defaultToOrchestrator: Boolean = false, test: (ClientCodegenContext, RustCrate) -> Unit = { _, _ -> }, ) = clientIntegrationTest( @@ -58,6 +62,9 @@ fun awsSdkIntegrationTest( "codegen", ObjectNode.builder() .withMember("includeFluentClient", false) + .letIf(defaultToOrchestrator) { + it.withMember("enableNewSmithyRuntime", StringNode.from("orchestrator")) + } .build(), ).build(), ), diff --git a/aws/sdk/integration-tests/s3/tests/config-override.rs b/aws/sdk/integration-tests/s3/tests/config-override.rs new file mode 100644 index 00000000000..b3d93be8676 --- /dev/null +++ b/aws/sdk/integration-tests/s3/tests/config-override.rs @@ -0,0 +1,121 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use aws_credential_types::provider::SharedCredentialsProvider; +use aws_sdk_s3::config::{Credentials, Region}; +use aws_sdk_s3::Client; +use aws_smithy_client::test_connection::{capture_request, CaptureRequestReceiver}; +use aws_types::SdkConfig; + +// TODO(enableNewSmithyRuntimeCleanup): Remove this attribute once #[cfg(aws_sdk_orchestrator_mode)] +// has been removed +#[allow(dead_code)] +fn test_client() -> (CaptureRequestReceiver, Client) { + let (conn, captured_request) = capture_request(None); + let sdk_config = SdkConfig::builder() + .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) + .region(Region::new("us-west-2")) + .http_connector(conn) + .build(); + let client = Client::new(&sdk_config); + (captured_request, client) +} + +#[cfg(aws_sdk_orchestrator_mode)] +#[tokio::test] +async fn operation_overrides_force_path_style() { + let (captured_request, client) = test_client(); + let _ = client + .list_objects_v2() + .bucket("test-bucket") + .customize() + .await + .unwrap() + .config_override(aws_sdk_s3::config::Config::builder().force_path_style(true)) + .send() + .await; + assert_eq!( + captured_request.expect_request().uri().to_string(), + "https://s3.us-west-2.amazonaws.com/test-bucket/?list-type=2" + ); +} + +#[cfg(aws_sdk_orchestrator_mode)] +#[tokio::test] +async fn operation_overrides_fips() { + let (captured_request, client) = test_client(); + let _ = client + .list_objects_v2() + .bucket("test-bucket") + .customize() + .await + .unwrap() + .config_override(aws_sdk_s3::config::Config::builder().use_fips(true)) + .send() + .await; + assert_eq!( + captured_request.expect_request().uri().to_string(), + "https://test-bucket.s3-fips.us-west-2.amazonaws.com/?list-type=2" + ); +} + +#[cfg(aws_sdk_orchestrator_mode)] +#[tokio::test] +async fn operation_overrides_dual_stack() { + let (captured_request, client) = test_client(); + let _ = client + .list_objects_v2() + .bucket("test-bucket") + .customize() + .await + .unwrap() + .config_override(aws_sdk_s3::config::Config::builder().use_dual_stack(true)) + .send() + .await; + assert_eq!( + captured_request.expect_request().uri().to_string(), + "https://test-bucket.s3.dualstack.us-west-2.amazonaws.com/?list-type=2" + ); +} + +// TODO(enableNewSmithyRuntimeCleanup): Comment in the following test once Handle is no longer +// accessed in ServiceRuntimePlugin::config. Currently, a credentials cache created for a single +// operation invocation is not picked up by an identity resolver. +/* +#[cfg(aws_sdk_orchestrator_mode)] +#[tokio::test] +async fn operation_overrides_credentials_provider() { + let (captured_request, client) = test_client(); + let _ = client + .list_objects_v2() + .bucket("test-bucket") + .customize() + .await + .unwrap() + .config_override(aws_sdk_s3::config::Config::builder().credentials_provider(Credentials::new( + "test", + "test", + Some("test".into()), + Some(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1669257290 + 3600)), + "test", + ))) + .request_time_for_tests(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1669257290)) + .send() + .await; + + let request = captured_request.expect_request(); + let actual_auth = + std::str::from_utf8(request.headers().get("authorization").unwrap().as_bytes()).unwrap(); + // signature would be f98cc3911dfba0daabf4343152f456bff9ecd3888a3068a1346d26949cb8f9e5 + // if we used `Credentials::for_tests()` + let expected_sig = "Signature=d7e7be63efc37c5bab5eda121999cd1c9a95efdde0cc1ce7c1b8761051cc3cbd"; + assert!( + actual_auth.contains(expected_sig), + "authorization header signature did not match expected signature: expected {} but not found in {}", + expected_sig, + actual_auth, + ); +} +*/ diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt index 2c80bfd0cb5..1153c277705 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpConnectorConfigDecorator.kt @@ -217,6 +217,45 @@ private class HttpConnectorConfigCustomization( } } + is ServiceConfig.OperationConfigOverride -> writable { + if (runtimeMode.defaultToOrchestrator) { + rustTemplate( + """ + if let #{Some}(http_connector) = + layer.load::<#{HttpConnector}>() + { + let sleep_impl = layer + .load::<#{SharedAsyncSleep}>() + .or_else(|| { + self.client_config + .load::<#{SharedAsyncSleep}>() + }) + .cloned(); + let timeout_config = layer + .load::<#{TimeoutConfig}>() + .or_else(|| { + self.client_config + .load::<#{TimeoutConfig}>() + }) + .expect("timeout config should be set either in `config_override` or in the client config"); + let connector_settings = + #{ConnectorSettings}::from_timeout_config( + timeout_config, + ); + if let #{Some}(conn) = http_connector.connector(&connector_settings, sleep_impl) { + let connection: #{DynConnector} = #{DynConnector}::new(#{DynConnectorAdapter}::new( + // TODO(enableNewSmithyRuntimeCleanup): Replace the tower-based DynConnector and remove DynConnectorAdapter when deleting the middleware implementation + conn + )); + layer.set_connector(connection); + } + } + """, + *codegenScope, + ) + } + } + else -> emptySection } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt index b193172b552..692b874517a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt @@ -172,7 +172,7 @@ class InterceptorConfigCustomization(codegenContext: ClientCodegenContext) : Con is ServiceConfig.RuntimePluginInterceptors -> rust( """ - ${section.interceptors}.extend(self.interceptors.iter().cloned()); + ${section.interceptors}.extend(${section.interceptorsField}.interceptors.iter().cloned()); """, ) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt index 9cb2f869503..dc20049c46f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ResiliencyConfigCustomization.kt @@ -377,7 +377,7 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon } if retry_config.mode() == #{RetryMode}::Adaptive { - if let Some(time_source) = layer.load::<#{SharedTimeSource}>().cloned() { + if let #{Some}(time_source) = layer.load::<#{SharedTimeSource}>().cloned() { let seconds_since_unix_epoch = time_source .now() .duration_since(#{SystemTime}::UNIX_EPOCH) @@ -396,6 +396,12 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon let token_bucket = TOKEN_BUCKET.get_or_init(token_bucket_partition, #{TokenBucket}::default); layer.store_put(token_bucket); layer.set_retry_strategy(#{DynRetryStrategy}::new(#{StandardRetryStrategy}::new(&retry_config))); + + // TODO(enableNewSmithyRuntimeCleanup): Should not need to provide a default once smithy-rs##2770 + // is resolved + if layer.load::<#{TimeoutConfig}>().is_none() { + layer.store_put(#{TimeoutConfig}::disabled()); + } """, *codegenScope, ) @@ -414,6 +420,24 @@ class ResiliencyConfigCustomization(private val codegenContext: ClientCodegenCon } } + is ServiceConfig.OperationConfigOverride -> { + if (runtimeMode.defaultToOrchestrator) { + rustTemplate( + """ + if let #{Some}(retry_config) = layer + .load::<#{RetryConfig}>() + .cloned() + { + layer.set_retry_strategy( + #{DynRetryStrategy}::new(#{StandardRetryStrategy}::new(&retry_config)) + ); + } + """, + *codegenScope, + ) + } + } + else -> emptySection } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt index a6b07ecaaad..a55c5b3e7d0 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointConfigCustomization.kt @@ -240,6 +240,24 @@ internal class EndpointConfigCustomization( } } + is ServiceConfig.OperationConfigOverride -> { + if (runtimeMode.defaultToOrchestrator) { + rustTemplate( + """ + if let #{Some}(resolver) = layer + .load::<$sharedEndpointResolver>() + .cloned() + { + let endpoint_resolver = #{DynEndpointResolver}::new( + #{DefaultEndpointResolver}::<#{Params}>::new(resolver)); + layer.set_endpoint_resolver(endpoint_resolver); + } + """, + *codegenScope, + ) + } + } + else -> emptySection } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGenerator.kt new file mode 100644 index 00000000000..edfe94af95b --- /dev/null +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGenerator.kt @@ -0,0 +1,82 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.generators + +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization +import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig +import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +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.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations + +class ConfigOverrideRuntimePluginGenerator( + codegenContext: ClientCodegenContext, +) { + private val moduleUseName = codegenContext.moduleUseName() + private val codegenScope = codegenContext.runtimeConfig.let { rc -> + val runtimeApi = RuntimeType.smithyRuntimeApi(rc) + val smithyTypes = RuntimeType.smithyTypes(rc) + arrayOf( + *RuntimeType.preludeScope, + "CloneableLayer" to smithyTypes.resolve("config_bag::CloneableLayer"), + "ConfigBagAccessors" to runtimeApi.resolve("client::config_bag_accessors::ConfigBagAccessors"), + "FrozenLayer" to smithyTypes.resolve("config_bag::FrozenLayer"), + "InterceptorRegistrar" to runtimeApi.resolve("client::interceptors::InterceptorRegistrar"), + "Layer" to smithyTypes.resolve("config_bag::Layer"), + "RuntimePlugin" to runtimeApi.resolve("client::runtime_plugin::RuntimePlugin"), + ) + } + + fun render(writer: RustWriter, customizations: List) { + writer.rustTemplate( + """ + /// A plugin that enables configuration for a single operation invocation + /// + /// The `config` method will return a `FrozenLayer` by storing values from `config_override`. + /// In the case of default values requested, they will be obtained from `client_config`. + ##[derive(Debug)] + pub(crate) struct ConfigOverrideRuntimePlugin { + pub(crate) config_override: Builder, + pub(crate) client_config: #{FrozenLayer}, + } + + impl #{RuntimePlugin} for ConfigOverrideRuntimePlugin { + fn config(&self) -> #{Option}<#{FrozenLayer}> { + use #{ConfigBagAccessors}; + + ##[allow(unused_mut)] + let layer: #{Layer} = self + .config_override + .inner + .clone() + .into(); + let mut layer = layer.with_name("$moduleUseName::config::ConfigOverrideRuntimePlugin"); + #{config} + + #{Some}(layer.freeze()) + } + + fn interceptors(&self, _interceptors: &mut #{InterceptorRegistrar}) { + #{interceptors} + } + } + + """, + *codegenScope, + "config" to writable { + writeCustomizations( + customizations, + ServiceConfig.OperationConfigOverride("layer"), + ) + }, + "interceptors" to writable { + writeCustomizations(customizations, ServiceConfig.RuntimePluginInterceptors("_interceptors", "self.config_override")) + }, + ) + } +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt index 5bf9c8615b5..a0087b3d2a3 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt @@ -155,11 +155,15 @@ open class OperationGenerator( pub(crate) fn operation_runtime_plugins( client_runtime_plugins: #{RuntimePlugins}, + client_config: &crate::config::Config, config_override: #{Option}, ) -> #{RuntimePlugins} { let mut runtime_plugins = client_runtime_plugins.with_operation_plugin(Self::new()); if let Some(config_override) = config_override { - runtime_plugins = runtime_plugins.with_operation_plugin(config_override); + runtime_plugins = runtime_plugins.with_operation_plugin(crate::config::ConfigOverrideRuntimePlugin { + config_override, + client_config: #{RuntimePlugin}::config(client_config).expect("frozen layer should exist in client config"), + }) } runtime_plugins #{additional_runtime_plugins} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGenerator.kt index 5e18a991146..3f86fe720d3 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGenerator.kt @@ -242,7 +242,8 @@ class PaginatorGenerator private constructor( """ let runtime_plugins = #{operation}::operation_runtime_plugins( handle.runtime_plugins.clone(), - None, + &handle.conf, + #{None}, ); """, *codegenScope, diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt index 571abbd5087..1c85d9047b0 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceGenerator.kt @@ -55,7 +55,8 @@ class ServiceGenerator( .render(this, decorator.serviceRuntimePluginCustomizations(codegenContext, emptyList())) serviceConfigGenerator.renderRuntimePluginImplForSelf(this) - serviceConfigGenerator.renderRuntimePluginImplForBuilder(this) + ConfigOverrideRuntimePluginGenerator(codegenContext) + .render(this, decorator.configCustomizations(codegenContext, listOf())) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index 3292ae10bbd..6fe9978f78d 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -518,7 +518,8 @@ class FluentClientGenerator( let input = self.inner.build().map_err(#{SdkError}::construction_failure)?; let runtime_plugins = #{Operation}::operation_runtime_plugins( self.handle.runtime_plugins.clone(), - self.config_override + &self.handle.conf, + self.config_override, ); #{Operation}::orchestrate(&runtime_plugins, input).await } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt index 69634adba42..148353269f4 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt @@ -13,7 +13,6 @@ import software.amazon.smithy.model.shapes.ServiceShape import software.amazon.smithy.model.traits.IdempotencyTokenTrait import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule -import software.amazon.smithy.rust.codegen.client.smithy.customizations.codegenScope import software.amazon.smithy.rust.codegen.client.smithy.customize.TestUtilFeature import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter @@ -92,20 +91,20 @@ sealed class ServiceConfig(name: String) : Section(name) { */ object BuilderBuild : ServiceConfig("BuilderBuild") - // TODO(enableNewSmithyRuntimeLaunch): This is temporary until config builder is backed by a CloneableLayer. - // It is needed because certain config fields appear explicitly regardless of the smithy runtime mode, e.g. - // interceptors. The [BuilderBuild] section is bifurcated depending on the runtime mode (in the orchestrator mode, - // storing a field into a frozen layer and in the middleware moving it into a corresponding service config field) - // so we need a different temporary section to always move a field from a builder to service config within the - // build method. + /** + * A section for customizing individual fields in the initializer of Config + */ object BuilderBuildExtras : ServiceConfig("BuilderBuildExtras") /** - * A section for setting up a field to be used by RuntimePlugin + * A section for setting up a field to be used by ConfigOverrideRuntimePlugin */ - data class RuntimePluginConfig(val cfg: String) : ServiceConfig("ToRuntimePlugin") + data class OperationConfigOverride(val cfg: String) : ServiceConfig("ToRuntimePlugin") - data class RuntimePluginInterceptors(val interceptors: String) : ServiceConfig("ToRuntimePluginInterceptors") + /** + * A section for appending additional runtime plugins, stored in [interceptorsField], to [interceptors] + */ + data class RuntimePluginInterceptors(val interceptors: String, val interceptorsField: String) : ServiceConfig("ToRuntimePluginInterceptors") /** * A section for extra functionality that needs to be defined with the config module @@ -262,7 +261,7 @@ fun standardConfigParam(param: ConfigParam, codegenContext: ClientCodegenContext } } - is ServiceConfig.RuntimePluginConfig -> emptySection + is ServiceConfig.OperationConfigOverride -> emptySection else -> emptySection } @@ -434,7 +433,10 @@ class ServiceConfigGenerator( // requiring that items created and stored _during_ the build method be `Clone`, since they // will soon be part of a `FrozenLayer` owned by the service config. So we will convert the // current `CloneableLayer` into a `Layer` that does not impose the `Clone` requirement. - let mut layer: #{Layer} = self.inner.into(); + let layer: #{Layer} = self + .inner + .into(); + let mut layer = layer.with_name("$moduleUseName::config::config"); """, *codegenScope, ) @@ -478,35 +480,9 @@ class ServiceConfigGenerator( """, *codegenScope, - "config" to writable { writeCustomizations(customizations, ServiceConfig.RuntimePluginConfig("cfg")) }, - "interceptors" to writable { - writeCustomizations(customizations, ServiceConfig.RuntimePluginInterceptors("_interceptors")) - }, - ) - } - - fun renderRuntimePluginImplForBuilder(writer: RustWriter) { - writer.rustTemplate( - """ - impl #{RuntimePlugin} for Builder { - fn config(&self) -> #{Option}<#{FrozenLayer}> { - // TODO(enableNewSmithyRuntimeLaunch): Put into `cfg` the fields in `self.config_override` that are not `None` - ##[allow(unused_mut)] - let mut cfg = #{CloneableLayer}::new("service config"); - #{config} - #{Some}(cfg.freeze()) - } - - fn interceptors(&self, _interceptors: &mut #{InterceptorRegistrar}) { - #{interceptors} - } - } - - """, - *codegenScope, - "config" to writable { writeCustomizations(customizations, ServiceConfig.RuntimePluginConfig("cfg")) }, + "config" to writable { writeCustomizations(customizations, ServiceConfig.OperationConfigOverride("cfg")) }, "interceptors" to writable { - writeCustomizations(customizations, ServiceConfig.RuntimePluginInterceptors("_interceptors")) + writeCustomizations(customizations, ServiceConfig.RuntimePluginInterceptors("_interceptors", "self")) }, ) } diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt new file mode 100644 index 00000000000..b1ee6311a69 --- /dev/null +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ConfigOverrideRuntimePluginGeneratorTest.kt @@ -0,0 +1,247 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.generators + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.client.testutil.TestCodegenSettings +import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest +import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.smithyRuntimeApiTestUtil +import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope +import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.testModule +import software.amazon.smithy.rust.codegen.core.testutil.tokioTest +import software.amazon.smithy.rust.codegen.core.testutil.unitTest + +internal class ConfigOverrideRuntimePluginGeneratorTest { + private val model = """ + namespace com.example + use aws.protocols#awsJson1_0 + + @awsJson1_0 + service HelloService { + operations: [SayHello], + version: "1" + } + + @optionalAuth + operation SayHello { input: TestInput } + structure TestInput { + foo: String, + } + """.asSmithyModel() + + @Test + fun `operation overrides endpoint resolver`() { + clientIntegrationTest( + model, + params = IntegrationTestParams(additionalSettings = TestCodegenSettings.orchestratorMode()), + ) { clientCodegenContext, rustCrate -> + val runtimeConfig = clientCodegenContext.runtimeConfig + val codegenScope = arrayOf( + *preludeScope, + "ConfigBagAccessors" to RuntimeType.configBagAccessors(runtimeConfig), + "EndpointResolverParams" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::orchestrator::EndpointResolverParams"), + "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), + ) + rustCrate.testModule { + addDependency(CargoDependency.Tokio.withFeature("test-util").toDevDependency()) + tokioTest("test_operation_overrides_endpoint_resolver") { + rustTemplate( + """ + use #{ConfigBagAccessors}; + use #{RuntimePlugin}; + + let expected_url = "http://localhost:1234/"; + let client_config = crate::config::Config::builder().build(); + let config_override = + crate::config::Config::builder().endpoint_resolver(expected_url); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config.config().unwrap(), + config_override, + }; + let sut_layer = sut.config().unwrap(); + let endpoint_resolver = sut_layer.endpoint_resolver(); + let endpoint = endpoint_resolver + .resolve_endpoint(&#{EndpointResolverParams}::new(crate::config::endpoint::Params {})) + .await + .unwrap(); + + assert_eq!(expected_url, endpoint.url()); + """, + *codegenScope, + ) + } + } + } + } + + @Test + fun `operation overrides http connector`() { + clientIntegrationTest( + model, + params = IntegrationTestParams(additionalSettings = TestCodegenSettings.orchestratorMode()), + ) { clientCodegenContext, rustCrate -> + val runtimeConfig = clientCodegenContext.runtimeConfig + val codegenScope = arrayOf( + *preludeScope, + "ConfigBagAccessors" to RuntimeType.configBagAccessors(runtimeConfig), + "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), + ) + rustCrate.testModule { + addDependency(CargoDependency.Tokio.withFeature("test-util").toDevDependency()) + tokioTest("test_operation_overrides_http_connection") { + rustTemplate( + """ + use #{AsyncSleep}; + + let (conn, captured_request) = #{capture_request}(#{None}); + let expected_url = "http://localhost:1234/"; + let client_config = crate::config::Config::builder() + .endpoint_resolver(expected_url) + .http_connector(#{NeverConnector}::new()) + .build(); + let client = crate::client::Client::from_conf(client_config.clone()); + let sleep = #{TokioSleep}::new(); + + let send = client.say_hello().send(); + let timeout = #{Timeout}::new( + send, + sleep.sleep(::std::time::Duration::from_millis(100)), + ); + + // sleep future should win because the other future `send` is backed by `NeverConnector`, + // yielding `Err` + matches!(timeout.await, Err(_)); + + let client = crate::client::Client::from_conf(client_config); + let customizable_send = client + .say_hello() + .customize() + .await + .unwrap() + .config_override(crate::config::Config::builder().http_connector(conn)) + .send(); + + let timeout = #{Timeout}::new( + customizable_send, + sleep.sleep(::std::time::Duration::from_millis(100)), + ); + + // `conn` should shadow `NeverConnector` by virtue of `config_override` + match timeout.await { + Ok(_) => { + assert_eq!( + expected_url, + captured_request.expect_request().uri().to_string() + ); + } + Err(_) => { + panic!("this arm should not be reached since the `customizable_send` future should win"); + } + } + """, + *codegenScope, + "AsyncSleep" to RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep::AsyncSleep"), + "capture_request" to RuntimeType.captureRequest(runtimeConfig), + "NeverConnector" to RuntimeType.smithyClient(runtimeConfig) + .resolve("never::NeverConnector"), + "Timeout" to RuntimeType.smithyAsync(runtimeConfig).resolve("future::timeout::Timeout"), + "TokioSleep" to RuntimeType.smithyAsync(runtimeConfig) + .resolve("rt::sleep::TokioSleep"), + ) + } + } + } + } + + @Test + fun `operation overrides retry strategy`() { + clientIntegrationTest( + model, + params = IntegrationTestParams(additionalSettings = TestCodegenSettings.orchestratorMode()), + ) { clientCodegenContext, rustCrate -> + val runtimeConfig = clientCodegenContext.runtimeConfig + val codegenScope = arrayOf( + *preludeScope, + "AlwaysRetry" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::retries::AlwaysRetry"), + "ConfigBag" to RuntimeType.smithyTypes(runtimeConfig).resolve("config_bag::ConfigBag"), + "ConfigBagAccessors" to RuntimeType.configBagAccessors(runtimeConfig), + "ErrorKind" to RuntimeType.smithyTypes(runtimeConfig).resolve("retry::ErrorKind"), + "InterceptorContext" to RuntimeType.interceptorContext(runtimeConfig), + "Layer" to RuntimeType.smithyTypes(runtimeConfig).resolve("config_bag::Layer"), + "OrchestratorError" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::orchestrator::OrchestratorError"), + "RetryConfig" to RuntimeType.smithyTypes(clientCodegenContext.runtimeConfig) + .resolve("retry::RetryConfig"), + "RequestAttempts" to smithyRuntimeApiTestUtil(runtimeConfig).toType() + .resolve("client::request_attempts::RequestAttempts"), + "RetryClassifiers" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::retries::RetryClassifiers"), + "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), + "ShouldAttempt" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::retries::ShouldAttempt"), + "TypeErasedBox" to RuntimeType.smithyTypes(runtimeConfig).resolve("type_erasure::TypeErasedBox"), + ) + rustCrate.testModule { + unitTest("test_operation_overrides_retry_strategy") { + rustTemplate( + """ + use #{ConfigBagAccessors}; + use #{RuntimePlugin}; + + let client_config = crate::config::Config::builder() + .retry_config(#{RetryConfig}::standard().with_max_attempts(3)) + .build(); + + let client_config_layer = client_config.config().unwrap(); + + let mut ctx = #{InterceptorContext}::new(#{TypeErasedBox}::new(())); + ctx.set_output_or_error(#{Err}(#{OrchestratorError}::other("doesn't matter"))); + let mut layer = #{Layer}::new("test"); + layer.store_put(#{RequestAttempts}::new(1)); + layer.set_retry_classifiers( + #{RetryClassifiers}::new().with_classifier(#{AlwaysRetry}(#{ErrorKind}::TransientError)), + ); + + let mut cfg = #{ConfigBag}::of_layers(vec![layer]); + cfg.push_shared_layer(client_config_layer.clone()); + + let retry = cfg.retry_strategy().unwrap(); + assert!(matches!( + retry.should_attempt_retry(&ctx, &cfg).unwrap(), + #{ShouldAttempt}::YesAfterDelay(_) + )); + + // sets `max_attempts` to 1 implicitly by using `disabled()`, forcing it to run out of + // attempts with respect to `RequestAttempts` set to 1 above + let config_override = crate::config::Config::builder() + .retry_config(#{RetryConfig}::disabled()); + let sut = crate::config::ConfigOverrideRuntimePlugin { + client_config: client_config_layer, + config_override, + }; + let sut_layer = sut.config().unwrap(); + cfg.push_shared_layer(sut_layer); + let retry = cfg.retry_strategy().unwrap(); + + assert!(matches!( + retry.should_attempt_retry(&ctx, &cfg).unwrap(), + #{ShouldAttempt}::No + )); + """, + *codegenScope, + ) + } + } + } + } +} diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGeneratorTest.kt index 97f114fda0d..0726c6870c3 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/PaginatorGeneratorTest.kt @@ -6,8 +6,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators import org.junit.jupiter.api.Test -import software.amazon.smithy.model.node.ObjectNode -import software.amazon.smithy.model.node.StringNode +import software.amazon.smithy.rust.codegen.client.testutil.TestCodegenSettings import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.rust @@ -83,19 +82,11 @@ internal class PaginatorGeneratorTest { } } - private fun enableNewSmithyRuntime(): ObjectNode = ObjectNode.objectNodeBuilder() - .withMember( - "codegen", - ObjectNode.objectNodeBuilder() - .withMember("enableNewSmithyRuntime", StringNode.from("orchestrator")).build(), - ) - .build() - @Test fun `generate paginators that compile`() { clientIntegrationTest( model, - params = IntegrationTestParams(additionalSettings = enableNewSmithyRuntime()), + params = IntegrationTestParams(additionalSettings = TestCodegenSettings.orchestratorMode()), ) { clientCodegenContext, rustCrate -> rustCrate.integrationTest("paginators_generated") { Attribute.AllowUnusedImports.render(this) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt index d850b2cc264..b91020fe3fd 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt @@ -289,6 +289,8 @@ data class CargoDependency( fun smithyQuery(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-query") fun smithyRuntime(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-runtime") fun smithyRuntimeApi(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-runtime-api") + fun smithyRuntimeApiTestUtil(runtimeConfig: RuntimeConfig) = + smithyRuntimeApi(runtimeConfig).toDevDependency().withFeature("test-util") fun smithyTypes(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-types") fun smithyXml(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-xml") diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt index fa98e87fcb7..b40d5b8a062 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt @@ -215,10 +215,12 @@ fun RustWriter.unitTest( name: String, vararg args: Any, attribute: Attribute = Attribute.Test, + additionalAttributes: List = emptyList(), async: Boolean = false, block: Writable, ): RustWriter { attribute.render(this) + additionalAttributes.forEach { it.render(this) } if (async) { rust("async") } diff --git a/rust-runtime/aws-smithy-types/src/config_bag.rs b/rust-runtime/aws-smithy-types/src/config_bag.rs index 8a993e31149..12fefbe3b13 100644 --- a/rust-runtime/aws-smithy-types/src/config_bag.rs +++ b/rust-runtime/aws-smithy-types/src/config_bag.rs @@ -242,6 +242,13 @@ impl Layer { } } + pub fn with_name(self, name: impl Into>) -> Self { + Self { + name: name.into(), + props: self.props, + } + } + /// Load a storable item from the bag pub fn load(&self) -> ::ReturnedType<'_> { T::Storer::merge_iter(ItemIter { From ddba46086a9754c01f3b11d7521c49d4489de84b Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 4 Jul 2023 14:19:15 +0200 Subject: [PATCH 6/6] Better distinguish model and HTTP plugins (#2827) So far, servers have tacitly worked with the notion that plugins come in two flavors: "HTTP plugins" and "model plugins": - A HTTP plugin acts on the HTTP request before it is deserialized, and acts on the HTTP response after it is serialized. - A model plugin acts on the modeled operation input after it is deserialized, and acts on the modeled operation output or the modeled operation error before it is serialized. However, this notion was never reified in the type system. Thus, users who pass in a model plugin where a HTTP plugin is expected or viceversa in several APIs: ```rust let pipeline = PluginPipeline::new().push(http_plugin).push(model_plugin); let app = SimpleService::builder_with_plugins(http_plugins, IdentityPlugin) .post_operation(handler) /* ... */ .build() .unwrap(); ``` would get the typical Tower service compilation errors, which can get very confusing: ``` error[E0631]: type mismatch in function arguments --> simple/rust-server-codegen/src/main.rs:47:6 | 15 | fn new_svc(inner: S) -> model_plugin::PostOperationService { | -------------------------------------------------------------------------- found signature defined here ... 47 | .post_operation(post_operation) | ^^^^^^^^^^^^^^ expected due to this | = note: expected function signature `fn(Upgrade, _>>) -> _` found function signature `fn(aws_smithy_http_server::operation::IntoService) -> _` = note: required for `LayerFn) -> ... {new_svc::<..., ...>}>` to implement `Layer, _>>>` = note: the full type name has been written to '/local/home/davidpz/workplace/smithy-ws/src/SmithyRsSource/target/debug/deps/simple-6577f9f79749ceb9.long-type-4938700695428041031.txt' ``` This commit introduces the `HttpPlugin` and `ModelPlugin` marker traits, allowing plugins to be marked as an HTTP plugin, a model plugin, or both. It also removes the primary way of concatenating plugins, `PluginPipeline`, in favor of `HttpPlugins` and `ModelPlugins`, which eagerly check that whenever a plugin is `push`ed, it is of the expected type. The generated service type in the server SDKs' `builder_with_plugins` constructor now takes an `HttpPlugin` as its first parameter, and a `ModelPlugin` as its second parameter. I think this change perhaps goes counter to the generally accepted wisdom that trait bounds in Rust should be enforced "at the latest possible moment", that is, only when the behavior encoded in the trait implementation is relied upon in the code (citation needed). However, the result is that exposing the concepts of HTTP plugin and model plugin in the type system makes for code that is easier to reason about, and produces more helpful compiler error messages. Documentation about the plugin system has been expanded, particularly on how to implement model plugins. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or 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._ --- CHANGELOG.next.toml | 40 ++- .../smithy/generators/ServerRootGenerator.kt | 9 +- .../ServerRuntimeTypesReExportsGenerator.kt | 7 +- .../generators/ServerServiceGenerator.kt | 35 +- design/src/server/anatomy.md | 32 +- design/src/server/instrumentation.md | 4 +- design/src/server/middleware.md | 28 +- .../tests/plugins_execution_order.rs | 16 +- examples/pokemon-service/src/main.rs | 6 +- examples/pokemon-service/src/plugin.rs | 13 +- .../aws-smithy-http-server/src/extension.rs | 14 +- .../src/instrumentation/plugin.rs | 10 +- .../src/plugin/alb_health_check.rs | 4 +- .../src/plugin/filter.rs | 11 +- .../plugin/{pipeline.rs => http_plugins.rs} | 113 ++++--- .../src/plugin/identity.rs | 6 +- .../src/plugin/layer.rs | 8 +- .../aws-smithy-http-server/src/plugin/mod.rs | 305 +++++++++++++++++- .../src/plugin/model_plugins.rs | 94 ++++++ .../src/plugin/scoped.rs | 7 +- .../src/plugin/stack.rs | 21 +- 21 files changed, 627 insertions(+), 156 deletions(-) rename rust-runtime/aws-smithy-http-server/src/plugin/{pipeline.rs => http_plugins.rs} (50%) create mode 100644 rust-runtime/aws-smithy-http-server/src/plugin/model_plugins.rs diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 35349116c0b..f3ac9bdc438 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -210,6 +210,7 @@ message = """The middleware system has been reworked as we push for a unified, s - A `ServiceShape` trait has been added. - The `Plugin` trait has been simplified. +- The `HttpMarker` and `ModelMarker` marker traits have been added to better distinguish when plugins run and what they have access to. - The `Operation` structure has been removed. - A `Scoped` `Plugin` has been added. @@ -371,6 +372,8 @@ where PrintService { inner, name: Op::ID.name() } } } + +impl HttpMarker for PrintPlugin { } ``` Alternatively, using the new `ServiceShape`, implemented on `Ser`: @@ -397,6 +400,11 @@ let app = PokemonService::builder_with_plugins(/* HTTP plugins */, /* model plug .unwrap(); ``` +To better distinguish when a plugin runs and what it has access to, `Plugin`s now have to additionally implement the `HttpMarker` marker trait, the `ModelMarker` marker trait, or both: + +- A HTTP plugin acts on the HTTP request before it is deserialized, and acts on the HTTP response after it is serialized. +- A model plugin acts on the modeled operation input after it is deserialized, and acts on the modeled operation output or the modeled operation error before it is serialized. + The motivation behind this change is to simplify the job of middleware authors, separate concerns, accomodate common cases better, and to improve composition internally. Because `Plugin` is now closer to `tower::Layer` we have two canonical converters: @@ -413,6 +421,34 @@ let plugin = /* some plugin */; let layer = LayerPlugin::new::(plugin); ``` +## Removal of `PluginPipeline` + +Since plugins now come in two flavors (those marked with `HttpMarker` and those marked with `ModelMarker`) that shouldn't be mixed in a collection of plugins, the primary way of concatenating plugins, `PluginPipeline` has been removed in favor of the `HttpPlugins` and `ModelPlugins` types, which eagerly check that whenever a plugin is pushed, it is of the expected type. + +This worked before, but you wouldn't be able to do apply this collection of plugins anywhere; if you tried to, the compilation error messages would not be very helpful: + +```rust +use aws_smithy_http_server::plugin::PluginPipeline; + +let pipeline = PluginPipeline::new().push(http_plugin).push(model_plugin); +``` + +Now collections of plugins must contain plugins of the same flavor: + +```rust +use aws_smithy_http_server::plugin::{HttpPlugins, ModelPlugins}; + +let http_plugins = HttpPlugins::new() + .push(http_plugin) + // .push(model_plugin) // This fails to compile with a helpful error message. + .push(&http_and_model_plugin); +let model_plugins = ModelPlugins::new() + .push(model_plugin) + .push(&http_and_model_plugin); +``` + +In the above example, `&http_and_model_plugin` implements both `HttpMarker` and `ModelMarker`, so we can add it to both collections. + ## Removal of `Operation` The `aws_smithy_http_server::operation::Operation` structure has now been removed. Previously, there existed a `{operation_name}_operation` setter on the service builder, which accepted an `Operation`. This allowed users to @@ -495,8 +531,8 @@ let scoped_plugin = Scoped::new::(plugin); ``` """ -references = ["smithy-rs#2740", "smithy-rs#2759", "smithy-rs#2779"] -meta = { "breaking" = true, "tada" = false, "bug" = false } +references = ["smithy-rs#2740", "smithy-rs#2759", "smithy-rs#2779", "smithy-rs#2827"] +meta = { "breaking" = true, "tada" = true, "bug" = false } author = "hlbarber" [[smithy-rs]] diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRootGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRootGenerator.kt index c8c1dd8450a..e300d248b17 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRootGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRootGenerator.kt @@ -106,7 +106,8 @@ open class ServerRootGenerator( //! #### Plugins //! //! The [`$serviceName::builder_with_plugins`] method, returning [`$builderName`], - //! accepts a [`Plugin`](aws_smithy_http_server::plugin::Plugin). + //! accepts a plugin marked with [`HttpMarker`](aws_smithy_http_server::plugin::HttpMarker) and a + //! plugin marked with [`ModelMarker`](aws_smithy_http_server::plugin::ModelMarker). //! Plugins allow you to build middleware which is aware of the operation it is being applied to. //! //! ```rust @@ -114,13 +115,13 @@ open class ServerRootGenerator( //! ## use #{SmithyHttpServer}::plugin::IdentityPlugin as LoggingPlugin; //! ## use #{SmithyHttpServer}::plugin::IdentityPlugin as MetricsPlugin; //! ## use #{Hyper}::Body; - //! use #{SmithyHttpServer}::plugin::PluginPipeline; + //! use #{SmithyHttpServer}::plugin::HttpPlugins; //! use $crateName::{$serviceName, $builderName}; //! - //! let plugins = PluginPipeline::new() + //! let http_plugins = HttpPlugins::new() //! .push(LoggingPlugin) //! .push(MetricsPlugin); - //! let builder: $builderName = $serviceName::builder_with_plugins(plugins, IdentityPlugin); + //! let builder: $builderName = $serviceName::builder_with_plugins(http_plugins, IdentityPlugin); //! ``` //! //! Check out [`#{SmithyHttpServer}::plugin`] to learn more about plugins. diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRuntimeTypesReExportsGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRuntimeTypesReExportsGenerator.kt index 85a95076484..a6f62246d87 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRuntimeTypesReExportsGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerRuntimeTypesReExportsGenerator.kt @@ -9,14 +9,12 @@ import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.server.smithy.ServerCargoDependency -import software.amazon.smithy.rust.codegen.server.smithy.ServerRuntimeType class ServerRuntimeTypesReExportsGenerator( codegenContext: CodegenContext, ) { private val runtimeConfig = codegenContext.runtimeConfig private val codegenScope = arrayOf( - "Router" to ServerRuntimeType.router(runtimeConfig), "SmithyHttpServer" to ServerCargoDependency.smithyHttpServer(runtimeConfig).toType(), ) @@ -30,8 +28,11 @@ class ServerRuntimeTypesReExportsGenerator( pub use #{SmithyHttpServer}::operation::OperationShape; } pub mod plugin { + pub use #{SmithyHttpServer}::plugin::HttpPlugins; + pub use #{SmithyHttpServer}::plugin::ModelPlugins; + pub use #{SmithyHttpServer}::plugin::HttpMarker; + pub use #{SmithyHttpServer}::plugin::ModelMarker; pub use #{SmithyHttpServer}::plugin::Plugin; - pub use #{SmithyHttpServer}::plugin::PluginPipeline; pub use #{SmithyHttpServer}::plugin::PluginStack; } pub mod request { diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt index 2852734b3fa..2b4e2ee2e98 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt @@ -136,7 +136,7 @@ class ServerServiceGenerator( where HandlerType: #{SmithyHttpServer}::operation::Handler, - ModelPlugin: #{SmithyHttpServer}::plugin::Plugin< + ModelPl: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, #{SmithyHttpServer}::operation::IntoService @@ -144,9 +144,9 @@ class ServerServiceGenerator( #{SmithyHttpServer}::operation::UpgradePlugin::: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, - ModelPlugin::Output + ModelPl::Output >, - HttpPlugin: #{SmithyHttpServer}::plugin::Plugin< + HttpPl: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, < @@ -154,13 +154,13 @@ class ServerServiceGenerator( as #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, - ModelPlugin::Output + ModelPl::Output > >::Output >, - HttpPlugin::Output: #{Tower}::Service<#{Http}::Request, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static, - >>::Future: Send + 'static, + HttpPl::Output: #{Tower}::Service<#{Http}::Request, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static, + >>::Future: Send + 'static, { use #{SmithyHttpServer}::operation::OperationShapeExt; @@ -199,7 +199,7 @@ class ServerServiceGenerator( where S: #{SmithyHttpServer}::operation::OperationService, - ModelPlugin: #{SmithyHttpServer}::plugin::Plugin< + ModelPl: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, #{SmithyHttpServer}::operation::Normalize @@ -207,9 +207,9 @@ class ServerServiceGenerator( #{SmithyHttpServer}::operation::UpgradePlugin::: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, - ModelPlugin::Output + ModelPl::Output >, - HttpPlugin: #{SmithyHttpServer}::plugin::Plugin< + HttpPl: #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, < @@ -217,13 +217,13 @@ class ServerServiceGenerator( as #{SmithyHttpServer}::plugin::Plugin< $serviceName, crate::operation_shape::$structName, - ModelPlugin::Output + ModelPl::Output > >::Output >, - HttpPlugin::Output: #{Tower}::Service<#{Http}::Request, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static, - >>::Future: Send + 'static, + HttpPl::Output: #{Tower}::Service<#{Http}::Request, Response = #{Http}::Response<#{SmithyHttpServer}::body::BoxBody>, Error = ::std::convert::Infallible> + Clone + Send + 'static, + >>::Future: Send + 'static, { use #{SmithyHttpServer}::operation::OperationShapeExt; @@ -394,7 +394,7 @@ class ServerServiceGenerator( /** Returns a `Writable` containing the builder struct definition and its implementations. */ private fun builder(): Writable = writable { - val builderGenerics = listOf(builderBodyGenericTypeName, "HttpPlugin", "ModelPlugin").joinToString(", ") + val builderGenerics = listOf(builderBodyGenericTypeName, "HttpPl", "ModelPl").joinToString(", ") rustTemplate( """ /// The service builder for [`$serviceName`]. @@ -402,8 +402,8 @@ class ServerServiceGenerator( /// Constructed via [`$serviceName::builder_with_plugins`] or [`$serviceName::builder_without_plugins`]. pub struct $builderName<$builderGenerics> { ${builderFields.joinToString(", ")}, - http_plugin: HttpPlugin, - model_plugin: ModelPlugin + http_plugin: HttpPl, + model_plugin: ModelPl } impl<$builderGenerics> $builderName<$builderGenerics> { @@ -473,9 +473,10 @@ class ServerServiceGenerator( /// /// Use [`$serviceName::builder_without_plugins`] if you don't need to apply plugins. /// - /// Check out [`PluginPipeline`](#{SmithyHttpServer}::plugin::PluginPipeline) if you need to apply + /// Check out [`HttpPlugins`](#{SmithyHttpServer}::plugin::HttpPlugins) and + /// [`ModelPlugins`](#{SmithyHttpServer}::plugin::ModelPlugins) if you need to apply /// multiple plugins. - pub fn builder_with_plugins(http_plugin: HttpPlugin, model_plugin: ModelPlugin) -> $builderName { + pub fn builder_with_plugins(http_plugin: HttpPl, model_plugin: ModelPl) -> $builderName { $builderName { #{NotSetFields:W}, http_plugin, diff --git a/design/src/server/anatomy.md b/design/src/server/anatomy.md index bc7a550c95d..d15f4fbcc04 100644 --- a/design/src/server/anatomy.md +++ b/design/src/server/anatomy.md @@ -466,40 +466,40 @@ stateDiagram-v2 The service builder API requires plugins to be specified upfront - they must be passed as an argument to `builder_with_plugins` and cannot be modified afterwards. You might find yourself wanting to apply _multiple_ plugins to your service. -This can be accommodated via [`PluginPipeline`]. +This can be accommodated via [`HttpPlugins`] and [`ModelPlugins`]. ```rust # extern crate aws_smithy_http_server; -use aws_smithy_http_server::plugin::PluginPipeline; +use aws_smithy_http_server::plugin::HttpPlugins; # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; -let pipeline = PluginPipeline::new().push(LoggingPlugin).push(MetricsPlugin); +let http_plugins = HttpPlugins::new().push(LoggingPlugin).push(MetricsPlugin); ``` The plugins' runtime logic is executed in registration order. In the example above, `LoggingPlugin` would run first, while `MetricsPlugin` is executed last. -If you are vending a plugin, you can leverage `PluginPipeline` as an extension point: you can add custom methods to it using an extension trait. +If you are vending a plugin, you can leverage `HttpPlugins` or `ModelPlugins` as an extension point: you can add custom methods to it using an extension trait. For example: ```rust # extern crate aws_smithy_http_server; -use aws_smithy_http_server::plugin::{PluginPipeline, PluginStack}; +use aws_smithy_http_server::plugin::{HttpPlugins, PluginStack}; # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; # use aws_smithy_http_server::plugin::IdentityPlugin as AuthPlugin; pub trait AuthPluginExt { - fn with_auth(self) -> PluginPipeline>; + fn with_auth(self) -> HttpPlugins>; } -impl AuthPluginExt for PluginPipeline { - fn with_auth(self) -> PluginPipeline> { +impl AuthPluginExt for HttpPlugins { + fn with_auth(self) -> HttpPlugins> { self.push(AuthPlugin) } } -let pipeline = PluginPipeline::new() +let http_plugins = HttpPlugins::new() .push(LoggingPlugin) // Our custom method! .with_auth(); @@ -518,15 +518,15 @@ You can create an instance of a service builder by calling either `builder_witho /// The service builder for [`PokemonService`]. /// /// Constructed via [`PokemonService::builder`]. -pub struct PokemonServiceBuilder { +pub struct PokemonServiceBuilder { capture_pokemon_operation: Option>, empty_operation: Option>, get_pokemon_species: Option>, get_server_statistics: Option>, get_storage: Option>, health_check_operation: Option>, - http_plugin: HttpPlugin, - model_plugin: ModelPlugin + http_plugin: HttpPl, + model_plugin: ModelPl, } ``` @@ -537,7 +537,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g where HandlerType:Handler, - ModelPlugin: Plugin< + ModelPl: Plugin< PokemonService, GetPokemonSpecies, IntoService @@ -547,7 +547,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g GetPokemonSpecies, ModelPlugin::Output >, - HttpPlugin: Plugin< + HttpPl: Plugin< PokemonService, GetPokemonSpecies, UpgradePlugin::::Output @@ -565,7 +565,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g where S: OperationService, - ModelPlugin: Plugin< + ModelPl: Plugin< PokemonService, GetPokemonSpecies, Normalize @@ -575,7 +575,7 @@ The builder has two setter methods for each [Smithy Operation](https://awslabs.g GetPokemonSpecies, ModelPlugin::Output >, - HttpPlugin: Plugin< + HttpPl: Plugin< PokemonService, GetPokemonSpecies, UpgradePlugin::::Output diff --git a/design/src/server/instrumentation.md b/design/src/server/instrumentation.md index dfb5ee52520..08c72a20980 100644 --- a/design/src/server/instrumentation.md +++ b/design/src/server/instrumentation.md @@ -66,11 +66,11 @@ This is enabled via the `instrument` method provided by the `aws_smithy_http_ser # let handler = |req: GetPokemonSpeciesInput| async { Result::::Ok(todo!()) }; use aws_smithy_http_server::{ instrumentation::InstrumentExt, - plugin::{IdentityPlugin, PluginPipeline} + plugin::{IdentityPlugin, HttpPlugins} }; use pokemon_service_server_sdk::PokemonService; -let http_plugins = PluginPipeline::new().instrument(); +let http_plugins = HttpPlugins::new().instrument(); let app = PokemonService::builder_with_plugins(http_plugins, IdentityPlugin) .get_pokemon_species(handler) /* ... */ diff --git a/design/src/server/middleware.md b/design/src/server/middleware.md index 409c8700fab..af394b1bcaa 100644 --- a/design/src/server/middleware.md +++ b/design/src/server/middleware.md @@ -226,7 +226,7 @@ scope! { // Construct `LoggingLayer`. let logging_plugin = LayerPlugin(LoggingLayer::new()); let logging_plugin = Scoped::new::(logging_plugin); -let http_plugins = PluginPipeline::new().push(logging_plugin); +let http_plugins = HttpPlugins::new().push(logging_plugin); let app /* : PokemonService> */ = PokemonService::builder_with_plugins(http_plugins, IdentityPlugin) .get_pokemon_species(handler) @@ -265,7 +265,7 @@ scope! { // Construct `BufferLayer`. let buffer_plugin = LayerPlugin(BufferLayer::new(3)); let buffer_plugin = Scoped::new::(buffer_plugin); -let model_plugins = PluginPipeline::new().push(buffer_plugin); +let model_plugins = ModelPlugins::new().push(buffer_plugin); let app /* : PokemonService> */ = PokemonService::builder_with_plugins(IdentityPlugin, model_plugins) .get_pokemon_species(handler) @@ -347,23 +347,24 @@ where } ``` -You can provide a custom method to add your plugin to a `PluginPipeline` via an extension trait: +You can provide a custom method to add your plugin to a collection of `HttpPlugins` or `ModelPlugins` via an extension trait. For example, for `HttpPlugins`: ```rust # extern crate aws_smithy_http_server; # pub struct PrintPlugin; -use aws_smithy_http_server::plugin::{PluginPipeline, PluginStack}; +# impl aws_smithy_http_server::plugin::HttpMarker for PrintPlugin { } +use aws_smithy_http_server::plugin::{HttpPlugins, PluginStack}; -/// This provides a [`print`](PrintExt::print) method on [`PluginPipeline`]. +/// This provides a [`print`](PrintExt::print) method on [`HttpPlugins`]. pub trait PrintExt { /// Causes all operations to print the operation name when called. /// /// This works by applying the [`PrintPlugin`]. - fn print(self) -> PluginPipeline>; + fn print(self) -> HttpPlugins>; } -impl PrintExt for PluginPipeline { - fn print(self) -> PluginPipeline> { +impl PrintExt for HttpPlugins { + fn print(self) -> HttpPlugins> { self.push(PrintPlugin) } } @@ -377,14 +378,15 @@ This allows for: # use aws_smithy_http_server::plugin::{PluginStack, Plugin}; # struct PrintPlugin; # impl Plugin for PrintPlugin { type Output = T; fn apply(&self, svc: T) -> Self::Output { svc }} -# trait PrintExt { fn print(self) -> PluginPipeline>; } -# impl PrintExt for PluginPipeline { fn print(self) -> PluginPipeline> { self.push(PrintPlugin) }} +# impl aws_smithy_http_server::plugin::HttpMarker for PrintPlugin { } +# trait PrintExt { fn print(self) -> HttpPlugins>; } +# impl PrintExt for HttpPlugins { fn print(self) -> HttpPlugins> { self.push(PrintPlugin) }} # use pokemon_service_server_sdk::{operation_shape::GetPokemonSpecies, input::*, output::*, error::*}; # let handler = |req: GetPokemonSpeciesInput| async { Result::::Ok(todo!()) }; -use aws_smithy_http_server::plugin::{IdentityPlugin, PluginPipeline}; +use aws_smithy_http_server::plugin::{IdentityPlugin, HttpPlugins}; use pokemon_service_server_sdk::PokemonService; -let http_plugins = PluginPipeline::new() +let http_plugins = HttpPlugins::new() // [..other plugins..] // The custom method! .print(); @@ -397,4 +399,4 @@ let app /* : PokemonService> */ = PokemonService::builder_with_plugins( ``` The custom `print` method hides the details of the `Plugin` trait from the average consumer. -They interact with the utility methods on `PluginPipeline` and enjoy the self-contained documentation. +They interact with the utility methods on `HttpPlugins` and enjoy the self-contained documentation. diff --git a/examples/pokemon-service-common/tests/plugins_execution_order.rs b/examples/pokemon-service-common/tests/plugins_execution_order.rs index de9f2632f1e..7a768cb0051 100644 --- a/examples/pokemon-service-common/tests/plugins_execution_order.rs +++ b/examples/pokemon-service-common/tests/plugins_execution_order.rs @@ -11,7 +11,7 @@ use std::{ }; use aws_smithy_http::body::SdkBody; -use aws_smithy_http_server::plugin::{IdentityPlugin, Plugin, PluginPipeline}; +use aws_smithy_http_server::plugin::{HttpMarker, HttpPlugins, IdentityPlugin, Plugin}; use tower::{Layer, Service}; use pokemon_service_client::{operation::do_nothing::DoNothingInput, Config}; @@ -34,13 +34,15 @@ async fn plugin_layers_are_executed_in_registration_order() { // We can then check the vector content to verify the invocation order let output = Arc::new(Mutex::new(Vec::new())); - let pipeline = PluginPipeline::new() + let http_plugins = HttpPlugins::new() .push(SentinelPlugin::new("first", output.clone())) .push(SentinelPlugin::new("second", output.clone())); - let mut app = - pokemon_service_server_sdk::PokemonService::builder_with_plugins(pipeline, IdentityPlugin) - .do_nothing(do_nothing) - .build_unchecked(); + let mut app = pokemon_service_server_sdk::PokemonService::builder_with_plugins( + http_plugins, + IdentityPlugin, + ) + .do_nothing(do_nothing) + .build_unchecked(); let request = DoNothingInput::builder() .build() .unwrap() @@ -77,6 +79,8 @@ impl Plugin for SentinelPlugin { } } +impl HttpMarker for SentinelPlugin {} + /// A [`Service`] that adds a print log. #[derive(Clone, Debug)] pub struct SentinelService { diff --git a/examples/pokemon-service/src/main.rs b/examples/pokemon-service/src/main.rs index b3aa767f50c..b7566baa2e6 100644 --- a/examples/pokemon-service/src/main.rs +++ b/examples/pokemon-service/src/main.rs @@ -10,7 +10,7 @@ use std::{net::SocketAddr, sync::Arc}; use aws_smithy_http_server::{ extension::OperationExtensionExt, instrumentation::InstrumentExt, - plugin::{alb_health_check::AlbHealthCheckLayer, IdentityPlugin, PluginPipeline, Scoped}, + plugin::{alb_health_check::AlbHealthCheckLayer, HttpPlugins, IdentityPlugin, Scoped}, request::request_id::ServerRequestIdProviderLayer, AddExtensionLayer, }; @@ -51,9 +51,9 @@ pub async fn main() { } } // Scope the `PrintPlugin`, defined in `plugin.rs`, to `PrintScope` - let print_plugin = Scoped::new::(PluginPipeline::new().print()); + let print_plugin = Scoped::new::(HttpPlugins::new().print()); - let plugins = PluginPipeline::new() + let plugins = HttpPlugins::new() // Apply the scoped `PrintPlugin` .push(print_plugin) // Apply the `OperationExtensionPlugin` defined in `aws_smithy_http_server::extension`. This allows other diff --git a/examples/pokemon-service/src/plugin.rs b/examples/pokemon-service/src/plugin.rs index e030d3a4727..971aebb0156 100644 --- a/examples/pokemon-service/src/plugin.rs +++ b/examples/pokemon-service/src/plugin.rs @@ -7,7 +7,7 @@ use aws_smithy_http_server::{ operation::OperationShape, - plugin::{Plugin, PluginPipeline, PluginStack}, + plugin::{HttpMarker, HttpPlugins, Plugin, PluginStack}, service::ServiceShape, shape_id::ShapeId, }; @@ -63,16 +63,19 @@ where } } } -/// This provides a [`print`](PrintExt::print) method on [`PluginPipeline`]. + +impl HttpMarker for PrintPlugin {} + +/// This provides a [`print`](PrintExt::print) method on [`HttpPlugins`]. pub trait PrintExt { /// Causes all operations to print the operation name when called. /// /// This works by applying the [`PrintPlugin`]. - fn print(self) -> PluginPipeline>; + fn print(self) -> HttpPlugins>; } -impl PrintExt for PluginPipeline { - fn print(self) -> PluginPipeline> { +impl PrintExt for HttpPlugins { + fn print(self) -> HttpPlugins> { self.push(PrintPlugin) } } diff --git a/rust-runtime/aws-smithy-http-server/src/extension.rs b/rust-runtime/aws-smithy-http-server/src/extension.rs index a2a54affb2b..17ff01e9619 100644 --- a/rust-runtime/aws-smithy-http-server/src/extension.rs +++ b/rust-runtime/aws-smithy-http-server/src/extension.rs @@ -28,7 +28,7 @@ use thiserror::Error; use tower::Service; use crate::operation::OperationShape; -use crate::plugin::{Plugin, PluginPipeline, PluginStack}; +use crate::plugin::{HttpMarker, HttpPlugins, Plugin, PluginStack}; use crate::shape_id::ShapeId; pub use crate::request::extension::{Extension, MissingExtension}; @@ -128,16 +128,18 @@ where } } -/// An extension trait on [`PluginPipeline`] allowing the application of [`OperationExtensionPlugin`]. +impl HttpMarker for OperationExtensionPlugin {} + +/// An extension trait on [`HttpPlugins`] allowing the application of [`OperationExtensionPlugin`]. /// /// See [`module`](crate::extension) documentation for more info. pub trait OperationExtensionExt { /// Apply the [`OperationExtensionPlugin`], which inserts the [`OperationExtension`] into every [`http::Response`]. - fn insert_operation_extension(self) -> PluginPipeline>; + fn insert_operation_extension(self) -> HttpPlugins>; } -impl OperationExtensionExt for PluginPipeline { - fn insert_operation_extension(self) -> PluginPipeline> { +impl OperationExtensionExt for HttpPlugins { + fn insert_operation_extension(self) -> HttpPlugins> { self.push(OperationExtensionPlugin) } } @@ -221,7 +223,7 @@ mod tests { } // Apply `Plugin`. - let plugins = PluginPipeline::new().insert_operation_extension(); + let plugins = HttpPlugins::new().insert_operation_extension(); // Apply `Plugin`s `Layer`. let layer = PluginLayer::new::(plugins); diff --git a/rust-runtime/aws-smithy-http-server/src/instrumentation/plugin.rs b/rust-runtime/aws-smithy-http-server/src/instrumentation/plugin.rs index ee88555d731..f2dcca9cac1 100644 --- a/rust-runtime/aws-smithy-http-server/src/instrumentation/plugin.rs +++ b/rust-runtime/aws-smithy-http-server/src/instrumentation/plugin.rs @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -use crate::plugin::{PluginPipeline, PluginStack}; +use crate::plugin::{HttpMarker, HttpPlugins, PluginStack}; use crate::{operation::OperationShape, plugin::Plugin}; use super::sensitivity::Sensitivity; @@ -27,17 +27,19 @@ where } } +impl HttpMarker for InstrumentPlugin {} + /// An extension trait for applying [`InstrumentPlugin`]. pub trait InstrumentExt { /// Applies an [`InstrumentOperation`] to every operation, respecting the [@sensitive] trait given on the input and /// output models. See [`InstrumentOperation`](super::InstrumentOperation) for more information. /// /// [@sensitive]: https://awslabs.github.io/smithy/2.0/spec/documentation-traits.html#sensitive-trait - fn instrument(self) -> PluginPipeline>; + fn instrument(self) -> HttpPlugins>; } -impl InstrumentExt for PluginPipeline { - fn instrument(self) -> PluginPipeline> { +impl InstrumentExt for HttpPlugins { + fn instrument(self) -> HttpPlugins> { self.push(InstrumentPlugin) } } diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs b/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs index c87e86619cd..47b5460a3f6 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs @@ -9,9 +9,9 @@ //! # Example //! //! ```no_run -//! # use aws_smithy_http_server::{body, plugin::{PluginPipeline, alb_health_check::AlbHealthCheckLayer}}; +//! # use aws_smithy_http_server::{body, plugin::{HttpPlugins, alb_health_check::AlbHealthCheckLayer}}; //! # use hyper::{Body, Response, StatusCode}; -//! let plugins = PluginPipeline::new() +//! let plugins = HttpPlugins::new() //! // Handle all `/ping` health check requests by returning a `200 OK`. //! .layer(AlbHealthCheckLayer::from_handler("/ping", |_req| async { //! StatusCode::OK diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/filter.rs b/rust-runtime/aws-smithy-http-server/src/plugin/filter.rs index 0e4a195e6dc..f29fac8e505 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/filter.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/filter.rs @@ -3,12 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -use super::{either::Either, IdentityPlugin}; +use super::{either::Either, IdentityPlugin, ModelMarker}; use crate::operation::OperationShape; use crate::service::ContainsOperation; -use super::Plugin; +use super::{HttpMarker, Plugin}; /// Filters the application of an inner [`Plugin`] using a predicate over the /// [`ServiceShape::Operations`](crate::service::ServiceShape::Operations). @@ -41,11 +41,14 @@ where } } +impl HttpMarker for FilterByOperation where Inner: HttpMarker {} +impl ModelMarker for FilterByOperation where Inner: ModelMarker {} + /// Filters the application of an inner [`Plugin`] using a predicate over the /// [`ServiceShape::Operations`](crate::service::ServiceShape::Operations). /// -/// Users should prefer [`Scoped`](crate::plugin::Scoped) and fallback to [`filter_by_operation`] in cases where -/// [`Plugin`] application must be decided at runtime. +/// Users should prefer [`Scoped`](crate::plugin::Scoped) and fallback to [`filter_by_operation`] +/// in cases where [`Plugin`] application must be decided at runtime. /// /// # Example /// diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs b/rust-runtime/aws-smithy-http-server/src/plugin/http_plugins.rs similarity index 50% rename from rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs rename to rust-runtime/aws-smithy-http-server/src/plugin/http_plugins.rs index acac792c3b8..63aece88a62 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/http_plugins.rs @@ -3,25 +3,28 @@ * SPDX-License-Identifier: Apache-2.0 */ +// If you make any updates to this file (including Rust docs), make sure you make them to +// `model_plugins.rs` too! + use crate::plugin::{IdentityPlugin, Plugin, PluginStack}; -use super::LayerPlugin; +use super::{HttpMarker, LayerPlugin}; -/// A wrapper struct for composing [`Plugin`]s. -/// It is used as input for the `builder_with_plugins` method on the generate service struct +/// A wrapper struct for composing HTTP plugins. +/// It can be used as input for the `builder_with_plugins` method on the generated service struct /// (e.g. `PokemonService::builder_with_plugins`). /// /// ## Applying plugins in a sequence /// -/// You can use the [`push`](PluginPipeline::push) method to apply a new plugin after the ones that +/// You can use the [`push`](HttpPlugins::push) method to apply a new HTTP plugin after the ones that /// have already been registered. /// /// ```rust -/// use aws_smithy_http_server::plugin::PluginPipeline; +/// use aws_smithy_http_server::plugin::HttpPlugins; /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; /// -/// let pipeline = PluginPipeline::new().push(LoggingPlugin).push(MetricsPlugin); +/// let http_plugins = HttpPlugins::new().push(LoggingPlugin).push(MetricsPlugin); /// ``` /// /// The plugins' runtime logic is executed in registration order. @@ -32,13 +35,14 @@ use super::LayerPlugin; /// From time to time, you might have a need to transform the entire pipeline that has been built /// so far - e.g. you only want to apply those plugins for a specific operation. /// -/// `PluginPipeline` is itself a [`Plugin`]: you can apply any transformation that expects a -/// [`Plugin`] to an entire pipeline. In this case, we want to use -/// [`filter_by_operation`](crate::plugin::filter_by_operation) to limit the scope of -/// the logging and metrics plugins to the `CheckHealth` operation: +/// `HttpPlugins` is itself a [`Plugin`]: you can apply any transformation that expects a +/// [`Plugin`] to an entire pipeline. In this case, we could use a [scoped +/// plugin](crate::plugin::Scoped) to limit the scope of the logging and metrics plugins to the +/// `CheckHealth` operation: /// /// ```rust -/// use aws_smithy_http_server::plugin::{filter_by_operation, PluginPipeline}; +/// use aws_smithy_http_server::scope; +/// use aws_smithy_http_server::plugin::{HttpPlugins, Scoped}; /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as AuthPlugin; @@ -49,91 +53,101 @@ use super::LayerPlugin; /// # impl CheckHealth { const ID: ShapeId = ShapeId::new("namespace#MyName", "namespace", "MyName"); } /// /// // The logging and metrics plugins will only be applied to the `CheckHealth` operation. -/// let plugin = PluginPipeline::new() -/// .push(LoggingPlugin) -/// .push(MetricsPlugin); -/// let filtered_plugin = filter_by_operation(plugin, |operation: Operation| operation == Operation::CheckHealth); -/// let pipeline = PluginPipeline::new() +/// let plugin = HttpPlugins::new() +/// .push(LoggingPlugin) +/// .push(MetricsPlugin); +/// +/// scope! { +/// struct OnlyCheckHealth { +/// includes: [CheckHealth], +/// excludes: [/* The rest of the operations go here */] +/// } +/// } +/// +/// let filtered_plugin = Scoped::new::(&plugin); +/// let http_plugins = HttpPlugins::new() /// .push(filtered_plugin) -/// // The auth plugin will be applied to all operations +/// // The auth plugin will be applied to all operations. /// .push(AuthPlugin); /// ``` /// -/// ## Concatenating two plugin pipelines +/// ## Concatenating two collections of HTTP plugins /// -/// `PluginPipeline` is a good way to bundle together multiple plugins, ensuring they are all +/// `HttpPlugins` is a good way to bundle together multiple plugins, ensuring they are all /// registered in the correct order. /// -/// Since `PluginPipeline` is itself a [`Plugin`], you can use the [`push`](PluginPipeline::push) to -/// append, at once, all the plugins in another pipeline to the current pipeline: +/// Since `HttpPlugins` is itself a HTTP plugin (it implements the `HttpMarker` trait), you can use +/// the [`push`](HttpPlugins::push) to append, at once, all the HTTP plugins in another +/// `HttpPlugins` to the current `HttpPlugins`: /// /// ```rust -/// use aws_smithy_http_server::plugin::{IdentityPlugin, PluginPipeline, PluginStack}; +/// use aws_smithy_http_server::plugin::{IdentityPlugin, HttpPlugins, PluginStack}; /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as AuthPlugin; /// -/// pub fn get_bundled_pipeline() -> PluginPipeline>> { -/// PluginPipeline::new().push(LoggingPlugin).push(MetricsPlugin) +/// pub fn get_bundled_http_plugins() -> HttpPlugins>> { +/// HttpPlugins::new().push(LoggingPlugin).push(MetricsPlugin) /// } /// -/// let pipeline = PluginPipeline::new() +/// let http_plugins = HttpPlugins::new() /// .push(AuthPlugin) -/// .push(get_bundled_pipeline()); +/// .push(get_bundled_http_plugins()); /// ``` /// -/// ## Providing custom methods on `PluginPipeline` +/// ## Providing custom methods on `HttpPlugins` /// -/// You use an **extension trait** to add custom methods on `PluginPipeline`. +/// You use an **extension trait** to add custom methods on `HttpPlugins`. /// /// This is a simple example using `AuthPlugin`: /// /// ```rust -/// use aws_smithy_http_server::plugin::{PluginPipeline, PluginStack}; +/// use aws_smithy_http_server::plugin::{HttpPlugins, PluginStack}; /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as AuthPlugin; /// /// pub trait AuthPluginExt { -/// fn with_auth(self) -> PluginPipeline>; +/// fn with_auth(self) -> HttpPlugins>; /// } /// -/// impl AuthPluginExt for PluginPipeline { -/// fn with_auth(self) -> PluginPipeline> { +/// impl AuthPluginExt for HttpPlugins { +/// fn with_auth(self) -> HttpPlugins> { /// self.push(AuthPlugin) /// } /// } /// -/// let pipeline = PluginPipeline::new() +/// let http_plugins = HttpPlugins::new() /// .push(LoggingPlugin) /// // Our custom method! /// .with_auth(); /// ``` -pub struct PluginPipeline

(pub(crate) P); +#[derive(Debug)] +pub struct HttpPlugins

(pub(crate) P); -impl Default for PluginPipeline { +impl Default for HttpPlugins { fn default() -> Self { Self(IdentityPlugin) } } -impl PluginPipeline { - /// Create an empty [`PluginPipeline`]. +impl HttpPlugins { + /// Create an empty [`HttpPlugins`]. /// - /// You can use [`PluginPipeline::push`] to add plugins to it. + /// You can use [`HttpPlugins::push`] to add plugins to it. pub fn new() -> Self { Self::default() } } -impl

PluginPipeline

{ - /// Apply a new plugin after the ones that have already been registered. +impl

HttpPlugins

{ + /// Apply a new HTTP plugin after the ones that have already been registered. /// /// ```rust - /// use aws_smithy_http_server::plugin::PluginPipeline; + /// use aws_smithy_http_server::plugin::HttpPlugins; /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; /// # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; /// - /// let pipeline = PluginPipeline::new().push(LoggingPlugin).push(MetricsPlugin); + /// let http_plugins = HttpPlugins::new().push(LoggingPlugin).push(MetricsPlugin); /// ``` /// /// The plugins' runtime logic is executed in registration order. @@ -163,18 +177,19 @@ impl

PluginPipeline

{ /// } /// } /// ``` - /// - pub fn push(self, new_plugin: NewPlugin) -> PluginPipeline> { - PluginPipeline(PluginStack::new(new_plugin, self.0)) + // We eagerly require `NewPlugin: HttpMarker`, despite not really needing it, because compiler + // errors get _substantially_ better if the user makes a mistake. + pub fn push(self, new_plugin: NewPlugin) -> HttpPlugins> { + HttpPlugins(PluginStack::new(new_plugin, self.0)) } /// Applies a single [`tower::Layer`] to all operations _before_ they are deserialized. - pub fn layer(self, layer: L) -> PluginPipeline, P>> { - PluginPipeline(PluginStack::new(LayerPlugin(layer), self.0)) + pub fn layer(self, layer: L) -> HttpPlugins, P>> { + HttpPlugins(PluginStack::new(LayerPlugin(layer), self.0)) } } -impl Plugin for PluginPipeline +impl Plugin for HttpPlugins where InnerPlugin: Plugin, { @@ -184,3 +199,5 @@ where self.0.apply(input) } } + +impl HttpMarker for HttpPlugins where InnerPlugin: HttpMarker {} diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/identity.rs b/rust-runtime/aws-smithy-http-server/src/plugin/identity.rs index affbd9f6b90..6ec684a5326 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/identity.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/identity.rs @@ -3,9 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -use super::Plugin; +use super::{HttpMarker, ModelMarker, Plugin}; /// A [`Plugin`] that maps a service to itself. +#[derive(Debug)] pub struct IdentityPlugin; impl Plugin for IdentityPlugin { @@ -15,3 +16,6 @@ impl Plugin for IdentityPlugin { svc } } + +impl ModelMarker for IdentityPlugin {} +impl HttpMarker for IdentityPlugin {} diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/layer.rs b/rust-runtime/aws-smithy-http-server/src/plugin/layer.rs index b1a025b4cb8..0a2f31bc485 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/layer.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/layer.rs @@ -7,7 +7,7 @@ use std::marker::PhantomData; use tower::Layer; -use super::Plugin; +use super::{HttpMarker, ModelMarker, Plugin}; /// A [`Plugin`] which acts as a [`Layer`] `L`. pub struct LayerPlugin(pub L); @@ -23,6 +23,12 @@ where } } +// Without more information about what the layer `L` does, we can't know whether it's appropriate +// to run this plugin as a HTTP plugin or a model plugin, so we implement both marker traits. + +impl HttpMarker for LayerPlugin {} +impl ModelMarker for LayerPlugin {} + /// A [`Layer`] which acts as a [`Plugin`] `Pl` for specific protocol `P` and operation `Op`. pub struct PluginLayer { plugin: Pl, diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs b/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs index 106dc9200c7..5bbeda3eba7 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs @@ -5,13 +5,42 @@ //! The plugin system allows you to build middleware with an awareness of the operation it is applied to. //! -//! The system centers around the [`Plugin`] trait. In addition, this module provides helpers for composing and -//! combining [`Plugin`]s. +//! The system centers around the [`Plugin`], [`HttpMarker`], and [`ModelMarker`] traits. In +//! addition, this module provides helpers for composing and combining [`Plugin`]s. +//! +//! # HTTP plugins vs model plugins +//! +//! Plugins come in two flavors: _HTTP_ plugins and _model_ plugins. The key difference between +//! them is _when_ they run: +//! +//! - A HTTP plugin acts on the HTTP request before it is deserialized, and acts on the HTTP response +//! after it is serialized. +//! - A model plugin acts on the modeled operation input after it is deserialized, and acts on the +//! modeled operation output or the modeled operation error before it is serialized. +//! +//! See the relevant section in [the book], which contains an illustrative diagram. +//! +//! Both kinds of plugins implement the [`Plugin`] trait, but only HTTP plugins implement the +//! [`HttpMarker`] trait and only model plugins implement the [`ModelMarker`] trait. There is no +//! difference in how an HTTP plugin or a model plugin is applied, so both the [`HttpMarker`] trait +//! and the [`ModelMarker`] trait are _marker traits_, they carry no behavior. Their only purpose +//! is to mark a plugin, at the type system leve, as allowed to run at a certain time. A plugin can be +//! _both_ a HTTP plugin and a model plugin by implementing both traits; in this case, when the +//! plugin runs is decided by you when you register it in your application. [`IdentityPlugin`], +//! [`Scoped`], and [`LayerPlugin`] are examples of plugins that implement both traits. +//! +//! In practice, most plugins are HTTP plugins. Since HTTP plugins run before a request has been +//! correctly deserialized, HTTP plugins should be fast and lightweight. Only use model plugins if +//! you absolutely require your middleware to run after deserialization, or to act on particular +//! fields of your deserialized operation's input/output/errors. +//! +//! [the book]: https://awslabs.github.io/smithy-rs/design/server/anatomy.html //! //! # Filtered application of a HTTP [`Layer`](tower::Layer) //! //! ``` //! # use aws_smithy_http_server::plugin::*; +//! # use aws_smithy_http_server::scope; //! # use aws_smithy_http_server::shape_id::ShapeId; //! # let layer = (); //! # #[derive(PartialEq)] @@ -21,8 +50,18 @@ //! // Create a `Plugin` from a HTTP `Layer` //! let plugin = LayerPlugin(layer); //! -//! // Only apply the layer to operations with name "GetPokemonSpecies" -//! let plugin = filter_by_operation(plugin, |operation: Operation| operation == Operation::GetPokemonSpecies); +//! scope! { +//! struct OnlyGetPokemonSpecies { +//! includes: [GetPokemonSpecies], +//! excludes: [/* The rest of the operations go here */] +//! } +//! } +//! +//! // Only apply the layer to operations with name "GetPokemonSpecies". +//! let filtered_plugin = Scoped::new::(&plugin); +//! +//! // The same effect can be achieved at runtime. +//! let filtered_plugin = filter_by_operation(&plugin, |operation: Operation| operation == Operation::GetPokemonSpecies); //! ``` //! //! # Construct a [`Plugin`] from a closure that takes as input the operation name @@ -68,25 +107,35 @@ //! //! # Combine [`Plugin`]s //! -//! ``` +//! ```no_run //! # use aws_smithy_http_server::plugin::*; -//! # let a = (); let b = (); -//! // Combine `Plugin`s `a` and `b` -//! let plugin = PluginPipeline::new() +//! # struct Foo; +//! # impl HttpMarker for Foo { } +//! # let a = Foo; let b = Foo; +//! // Combine `Plugin`s `a` and `b`. Both need to implement `HttpMarker`. +//! let plugin = HttpPlugins::new() //! .push(a) //! .push(b); //! ``` //! -//! As noted in the [`PluginPipeline`] documentation, the plugins' runtime logic is executed in registration order, +//! As noted in the [`HttpPlugins`] documentation, the plugins' runtime logic is executed in registration order, //! meaning that `a` is run _before_ `b` in the example above. //! -//! # Example Implementation +//! Similarly, you can use [`ModelPlugins`] to combine model plugins. //! -//! ```rust +//! # Example implementation of a [`Plugin`] +//! +//! The following is an example implementation of a [`Plugin`] that prints out the service's name +//! and the name of the operation that was hit every time it runs. Since it doesn't act on the HTTP +//! request nor the modeled operation input/output/errors, this plugin can be both an HTTP plugin +//! and a model plugin. In practice, however, you'd only want to register it once, as either an +//! HTTP plugin or a model plugin. +//! +//! ```no_run //! use aws_smithy_http_server::{ //! operation::OperationShape, //! service::ServiceShape, -//! plugin::{Plugin, PluginPipeline, PluginStack}, +//! plugin::{Plugin, HttpMarker, HttpPlugins, ModelMarker}, //! shape_id::ShapeId, //! }; //! # use tower::{layer::util::Stack, Layer, Service}; @@ -137,16 +186,22 @@ //! } //! } //! } -//! ``` //! +//! // This plugin could be registered as an HTTP plugin and a model plugin, so we implement both +//! // marker traits. +//! +//! impl HttpMarker for PrintPlugin { } +//! impl ModelMarker for PrintPlugin { } +//! ``` pub mod alb_health_check; mod closure; mod either; mod filter; +mod http_plugins; mod identity; mod layer; -mod pipeline; +mod model_plugins; #[doc(hidden)] pub mod scoped; mod stack; @@ -154,9 +209,10 @@ mod stack; pub use closure::{plugin_from_operation_fn, OperationFn}; pub use either::Either; pub use filter::{filter_by_operation, FilterByOperation}; +pub use http_plugins::HttpPlugins; pub use identity::IdentityPlugin; pub use layer::{LayerPlugin, PluginLayer}; -pub use pipeline::PluginPipeline; +pub use model_plugins::ModelPlugins; pub use scoped::Scoped; pub use stack::PluginStack; @@ -188,3 +244,222 @@ where >::apply(self, inner) } } + +/// A HTTP plugin is a plugin that acts on the HTTP request before it is deserialized, and acts on +/// the HTTP response after it is serialized. +/// +/// This trait is a _marker_ trait to indicate that a plugin can be registered as an HTTP plugin. +/// +/// Compare with [`ModelMarker`] in the [module](crate::plugin) documentation, which contains an +/// example implementation too. +pub trait HttpMarker {} +impl<'a, Pl> HttpMarker for &'a Pl where Pl: HttpMarker {} + +/// A model plugin is a plugin that acts on the modeled operation input after it is deserialized, +/// and acts on the modeled operation output or the modeled operation error before it is +/// serialized. +/// +/// This trait is a _marker_ trait to indicate that a plugin can be registered as a model plugin. +/// +/// Compare with [`HttpMarker`] in the [module](crate::plugin) documentation. +/// +/// # Example implementation of a model plugin +/// +/// Model plugins are most useful when you really need to rely on the actual shape of your +/// modeled operation input, operation output, and/or operation errors. For this reason, most +/// model plugins' implementation are _operation-specific_: somewhere in the type signature +/// of their definition, they'll rely on a operation shape's types. It is therefore important +/// that you scope application of model plugins to the operations they are meant to work on, via +/// [`Scoped`](crate::plugin::Scoped) or [`filter_by_operation`](crate::plugin::filter_by_operation). +/// +/// Below is an example implementation of a model plugin that can only be applied to the +/// `CheckHealth` operation: note how in the `Service` trait implementation, we require access to +/// the operation's input, where we log the `health_info` field. +/// +/// ```no_run +/// use std::marker::PhantomData; +/// +/// use aws_smithy_http_server::{operation::OperationShape, plugin::{ModelMarker, Plugin}}; +/// use tower::Service; +/// # pub struct SimpleService; +/// # pub struct CheckHealth; +/// # pub struct CheckHealthInput { +/// # health_info: (), +/// # } +/// # pub struct CheckHealthOutput; +/// # impl aws_smithy_http_server::operation::OperationShape for CheckHealth { +/// # const ID: aws_smithy_http_server::shape_id::ShapeId = aws_smithy_http_server::shape_id::ShapeId::new( +/// # "com.amazonaws.simple#CheckHealth", +/// # "com.amazonaws.simple", +/// # "CheckHealth", +/// # ); +/// # type Input = CheckHealthInput; +/// # type Output = CheckHealthOutput; +/// # type Error = std::convert::Infallible; +/// # } +/// +/// /// A model plugin that can only be applied to the `CheckHealth` operation. +/// pub struct CheckHealthPlugin { +/// pub _exts: PhantomData, +/// } +/// +/// impl CheckHealthPlugin { +/// pub fn new() -> Self { +/// Self { _exts: PhantomData } +/// } +/// } +/// +/// impl Plugin for CheckHealthPlugin { +/// type Output = CheckHealthService; +/// +/// fn apply(&self, input: T) -> Self::Output { +/// CheckHealthService { +/// inner: input, +/// _exts: PhantomData, +/// } +/// } +/// } +/// +/// impl ModelMarker for CheckHealthPlugin { } +/// +/// #[derive(Clone)] +/// pub struct CheckHealthService { +/// inner: S, +/// _exts: PhantomData, +/// } +/// +/// impl Service<(::Input, Exts)> for CheckHealthService +/// where +/// S: Service<(::Input, Exts)>, +/// { +/// type Response = S::Response; +/// type Error = S::Error; +/// type Future = S::Future; +/// +/// fn poll_ready(&mut self, cx: &mut std::task::Context<'_>) -> std::task::Poll> { +/// self.inner.poll_ready(cx) +/// } +/// +/// fn call(&mut self, req: (::Input, Exts)) -> Self::Future { +/// let (input, _exts) = &req; +/// +/// // We have access to `CheckHealth`'s modeled operation input! +/// dbg!(&input.health_info); +/// +/// self.inner.call(req) +/// } +/// } +/// +/// // In `main.rs` or wherever we register plugins, we have to make sure we only apply this plugin +/// // to the the only operation it can be applied to, the `CheckHealth` operation. If we apply the +/// // plugin to other operations, we will get a compilation error. +/// +/// use aws_smithy_http_server::plugin::Scoped; +/// use aws_smithy_http_server::scope; +/// +/// pub fn main() { +/// scope! { +/// struct OnlyCheckHealth { +/// includes: [CheckHealth], +/// excludes: [/* The rest of the operations go here */] +/// } +/// } +/// +/// let model_plugin = CheckHealthPlugin::new(); +/// # _foo(&model_plugin); +/// +/// // Scope the plugin to the `CheckHealth` operation. +/// let scoped_plugin = Scoped::new::(model_plugin); +/// # fn _foo(model_plugin: &CheckHealthPlugin<()>) {} +/// } +/// ``` +/// +/// If you are a service owner and don't care about giving a name to the model plugin, you can +/// simplify this down to: +/// +/// ```no_run +/// use std::marker::PhantomData; +/// +/// use aws_smithy_http_server::operation::OperationShape; +/// use tower::Service; +/// # pub struct SimpleService; +/// # pub struct CheckHealth; +/// # pub struct CheckHealthInput { +/// # health_info: (), +/// # } +/// # pub struct CheckHealthOutput; +/// # impl aws_smithy_http_server::operation::OperationShape for CheckHealth { +/// # const ID: aws_smithy_http_server::shape_id::ShapeId = aws_smithy_http_server::shape_id::ShapeId::new( +/// # "com.amazonaws.simple#CheckHealth", +/// # "com.amazonaws.simple", +/// # "CheckHealth", +/// # ); +/// # type Input = CheckHealthInput; +/// # type Output = CheckHealthOutput; +/// # type Error = std::convert::Infallible; +/// # } +/// +/// #[derive(Clone)] +/// pub struct CheckHealthService { +/// inner: S, +/// _exts: PhantomData, +/// } +/// +/// impl Service<(::Input, Exts)> for CheckHealthService +/// where +/// S: Service<(::Input, Exts)>, +/// { +/// type Response = S::Response; +/// type Error = S::Error; +/// type Future = S::Future; +/// +/// fn poll_ready(&mut self, cx: &mut std::task::Context<'_>) -> std::task::Poll> { +/// self.inner.poll_ready(cx) +/// } +/// +/// fn call(&mut self, req: (::Input, Exts)) -> Self::Future { +/// let (input, _exts) = &req; +/// +/// // We have access to `CheckHealth`'s modeled operation input! +/// dbg!(&input.health_info); +/// +/// self.inner.call(req) +/// } +/// } +/// +/// // In `main.rs`: +/// +/// use aws_smithy_http_server::plugin::LayerPlugin; +/// use aws_smithy_http_server::plugin::Scoped; +/// use aws_smithy_http_server::scope; +/// +/// fn new_check_health_service(inner: S) -> CheckHealthService { +/// CheckHealthService { +/// inner, +/// _exts: PhantomData, +/// } +/// } +/// +/// pub fn main() { +/// scope! { +/// struct OnlyCheckHealth { +/// includes: [CheckHealth], +/// excludes: [/* The rest of the operations go here */] +/// } +/// } +/// +/// # fn new_check_health_service(inner: ()) -> CheckHealthService<(), ()> { +/// # CheckHealthService { +/// # inner, +/// # _exts: PhantomData, +/// # } +/// # } +/// let layer = tower::layer::layer_fn(new_check_health_service); +/// let model_plugin = LayerPlugin(layer); +/// +/// // Scope the plugin to the `CheckHealth` operation. +/// let scoped_plugin = Scoped::new::(model_plugin); +/// } +/// ``` +pub trait ModelMarker {} +impl<'a, Pl> ModelMarker for &'a Pl where Pl: ModelMarker {} diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/model_plugins.rs b/rust-runtime/aws-smithy-http-server/src/plugin/model_plugins.rs new file mode 100644 index 00000000000..05dc8568d70 --- /dev/null +++ b/rust-runtime/aws-smithy-http-server/src/plugin/model_plugins.rs @@ -0,0 +1,94 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +// If you make any updates to this file (including Rust docs), make sure you make them to +// `http_plugins.rs` too! + +use crate::plugin::{IdentityPlugin, Plugin, PluginStack}; + +use super::{LayerPlugin, ModelMarker}; + +/// A wrapper struct for composing model plugins. +/// It operates identically to [`HttpPlugins`](crate::plugin::HttpPlugins); see its documentation. +#[derive(Debug)] +pub struct ModelPlugins

(pub(crate) P); + +impl Default for ModelPlugins { + fn default() -> Self { + Self(IdentityPlugin) + } +} + +impl ModelPlugins { + /// Create an empty [`ModelPlugins`]. + /// + /// You can use [`ModelPlugins::push`] to add plugins to it. + pub fn new() -> Self { + Self::default() + } +} + +impl

ModelPlugins

{ + /// Apply a new model plugin after the ones that have already been registered. + /// + /// ```rust + /// use aws_smithy_http_server::plugin::ModelPlugins; + /// # use aws_smithy_http_server::plugin::IdentityPlugin as LoggingPlugin; + /// # use aws_smithy_http_server::plugin::IdentityPlugin as MetricsPlugin; + /// + /// let model_plugins = ModelPlugins::new().push(LoggingPlugin).push(MetricsPlugin); + /// ``` + /// + /// The plugins' runtime logic is executed in registration order. + /// In our example above, `LoggingPlugin` would run first, while `MetricsPlugin` is executed last. + /// + /// ## Implementation notes + /// + /// Plugins are applied to the underlying [`Service`](tower::Service) in opposite order compared + /// to their registration order. + /// + /// As an example: + /// + /// ```rust,compile_fail + /// #[derive(Debug)] + /// pub struct PrintPlugin; + /// + /// impl Plugin for PrintPlugin + /// // [...] + /// { + /// // [...] + /// fn apply(&self, inner: T) -> Self::Service { + /// PrintService { + /// inner, + /// service_id: Ser::ID, + /// operation_id: Op::ID + /// } + /// } + /// } + /// ``` + // We eagerly require `NewPlugin: ModelMarker`, despite not really needing it, because compiler + // errors get _substantially_ better if the user makes a mistake. + pub fn push(self, new_plugin: NewPlugin) -> ModelPlugins> { + ModelPlugins(PluginStack::new(new_plugin, self.0)) + } + + /// Applies a single [`tower::Layer`] to all operations _before_ they are deserialized. + pub fn layer(self, layer: L) -> ModelPlugins, P>> { + ModelPlugins(PluginStack::new(LayerPlugin(layer), self.0)) + } +} + +impl Plugin for ModelPlugins +where + InnerPlugin: Plugin, +{ + type Output = InnerPlugin::Output; + + fn apply(&self, input: T) -> Self::Output { + self.0.apply(input) + } +} + +impl ModelMarker for ModelPlugins where InnerPlugin: ModelMarker {} diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/scoped.rs b/rust-runtime/aws-smithy-http-server/src/plugin/scoped.rs index a3761b2b1ab..d4b7e82e517 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/scoped.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/scoped.rs @@ -5,7 +5,7 @@ use std::marker::PhantomData; -use super::Plugin; +use super::{HttpMarker, ModelMarker, Plugin}; /// Marker struct for `true`. /// @@ -102,12 +102,15 @@ where } } +impl HttpMarker for Scoped where Pl: HttpMarker {} +impl ModelMarker for Scoped where Pl: ModelMarker {} + /// A macro to help with scoping [plugins](crate::plugin) to a subset of all operations. /// /// The scope must partition _all_ operations, that is, each and every operation must be included or excluded, but not /// both. /// -/// The generated server SDK exports a similar `scope` macro which is aware of a services operations and can complete +/// The generated server SDK exports a similar `scope` macro which is aware of a service's operations and can complete /// underspecified scopes automatically. /// /// # Example diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/stack.rs b/rust-runtime/aws-smithy-http-server/src/plugin/stack.rs index 63f8e448656..6c96ebaca00 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/stack.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/stack.rs @@ -3,13 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -use super::Plugin; +use super::{HttpMarker, ModelMarker, Plugin}; /// A wrapper struct which composes an `Inner` and an `Outer` [`Plugin`]. /// /// The `Inner::map` is run _then_ the `Outer::map`. /// -/// Note that the primary tool for composing plugins is [`PluginPipeline`](crate::plugin::PluginPipeline). +/// Note that the primary tool for composing HTTP plugins is +/// [`HttpPlugins`](crate::plugin::HttpPlugins), and the primary tool for composing HTTP plugins is +/// [`ModelPlugins`](crate::plugin::ModelPlugins); if you are an application writer, you should +/// prefer composing plugins using these. pub struct PluginStack { inner: Inner, outer: Outer, @@ -34,3 +37,17 @@ where self.outer.apply(svc) } } + +impl HttpMarker for PluginStack +where + Inner: HttpMarker, + Outer: HttpMarker, +{ +} + +impl ModelMarker for PluginStack +where + Inner: ModelMarker, + Outer: ModelMarker, +{ +}