From 26827f1acab8c061ec27dbfcc342648f93141668 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 13 Jul 2023 13:35:55 -0700 Subject: [PATCH] Don't allow `test-util` feature in compile time dependencies (#2843) During the orchestrator implementation, a bug was introduced that always enabled the `test-util` feature on `aws-smithy-runtime` in the generated crates. This led to several pieces of non-test code relying on test-only code, specifically around `SystemTime` implementing `TimeSource`. This PR fixes that issue, and also fixes another issue where the `RuntimeComponentsBuilder` was being initialized without a name for the service config. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../aws-config/src/provider_config.rs | 3 +- aws/rust-runtime/aws-config/src/sts/util.rs | 3 +- .../AwsCustomizableOperationDecorator.kt | 69 +++++++++++++++---- .../smithy/rustsdk/AwsPresigningDecorator.kt | 4 +- .../rustsdk/IntegrationTestDependencies.kt | 6 +- aws/sdk/integration-tests/s3/Cargo.toml | 2 +- .../config/ServiceConfigGenerator.kt | 24 ++++++- .../customizations/HttpAuthDecoratorTest.kt | 2 +- .../MetadataCustomizationTest.kt | 2 +- ...onfigOverrideRuntimePluginGeneratorTest.kt | 4 +- .../client/FluentClientGeneratorTest.kt | 2 + codegen-core/build.gradle.kts | 3 +- .../codegen/core/rustlang/CargoDependency.kt | 21 ++++-- .../rust/codegen/core/rustlang/RustWriter.kt | 9 +++ .../codegen/core/smithy/CodegenDelegator.kt | 7 +- .../smithy/generators/CargoTomlGenerator.kt | 44 +++++++++--- .../smithy/rust/codegen/core/testutil/Rust.kt | 37 ++++++---- .../core/rustlang/CargoDependencyTest.kt | 48 +++++++++++++ .../core/smithy/CodegenDelegatorTest.kt | 23 ++++++- .../src/client/runtime_components.rs | 6 +- .../src/client/orchestrator.rs | 2 +- .../src/client/test_util.rs | 1 - .../src/client/test_util/interceptors.rs | 45 ------------ 23 files changed, 255 insertions(+), 112 deletions(-) create mode 100644 codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependencyTest.kt delete mode 100644 rust-runtime/aws-smithy-runtime/src/client/test_util/interceptors.rs diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index afa2989811..27bc23108d 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -91,6 +91,7 @@ impl ProviderConfig { /// Unlike [`ProviderConfig::empty`] where `env` and `fs` will use their non-mocked implementations, /// this method will use an empty mock environment and an empty mock file system. pub fn no_configuration() -> Self { + use aws_smithy_async::time::StaticTimeSource; use std::collections::HashMap; use std::time::UNIX_EPOCH; let fs = Fs::from_raw_map(HashMap::new()); @@ -100,7 +101,7 @@ impl ProviderConfig { profile_files: ProfileFiles::default(), env, fs, - time_source: SharedTimeSource::new(UNIX_EPOCH), + time_source: SharedTimeSource::new(StaticTimeSource::new(UNIX_EPOCH)), connector: HttpConnector::Prebuilt(None), sleep: None, region: None, diff --git a/aws/rust-runtime/aws-config/src/sts/util.rs b/aws/rust-runtime/aws-config/src/sts/util.rs index c4fd630aef..bc6151985d 100644 --- a/aws/rust-runtime/aws-config/src/sts/util.rs +++ b/aws/rust-runtime/aws-config/src/sts/util.rs @@ -7,7 +7,6 @@ use aws_credential_types::provider::{self, error::CredentialsError}; use aws_credential_types::Credentials as AwsCredentials; use aws_sdk_sts::types::Credentials as StsCredentials; -use aws_smithy_async::time::TimeSource; use std::convert::TryFrom; use std::time::{SystemTime, UNIX_EPOCH}; @@ -47,6 +46,6 @@ pub(crate) fn into_credentials( /// provide a name for the session, the provider will choose a name composed of a base + a timestamp, /// e.g. `profile-file-provider-123456789` pub(crate) fn default_session_name(base: &str, ts: SystemTime) -> String { - let now = ts.now().duration_since(UNIX_EPOCH).expect("post epoch"); + let now = ts.duration_since(UNIX_EPOCH).expect("post epoch"); format!("{}-{}", base, now.as_millis()) } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCustomizableOperationDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCustomizableOperationDecorator.kt index 08d58134d4..6af6bfde0d 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCustomizableOperationDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCustomizableOperationDecorator.kt @@ -5,6 +5,7 @@ package software.amazon.smithy.rustsdk +import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.client.smithy.generators.client.CustomizableOperationCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.client.CustomizableOperationSection import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency @@ -18,28 +19,70 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) : CustomizableOperationCustomization() { private val codegenScope = arrayOf( *RuntimeType.preludeScope, - "AwsUserAgent" to AwsRuntimeType.awsHttp(runtimeConfig) - .resolve("user_agent::AwsUserAgent"), + "AwsUserAgent" to AwsRuntimeType.awsHttp(runtimeConfig).resolve("user_agent::AwsUserAgent"), "BeforeTransmitInterceptorContextMut" to RuntimeType.beforeTransmitInterceptorContextMut(runtimeConfig), "ConfigBag" to RuntimeType.configBag(runtimeConfig), "ConfigBagAccessors" to RuntimeType.configBagAccessors(runtimeConfig), "http" to CargoDependency.Http.toType(), "InterceptorContext" to RuntimeType.interceptorContext(runtimeConfig), - "StaticRuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig) - .resolve("client::runtime_plugin::StaticRuntimePlugin"), "RuntimeComponentsBuilder" to RuntimeType.runtimeComponentsBuilder(runtimeConfig), - "SharedTimeSource" to CargoDependency.smithyAsync(runtimeConfig).withFeature("test-util").toType() - .resolve("time::SharedTimeSource"), - "SharedInterceptor" to RuntimeType.smithyRuntimeApi(runtimeConfig) - .resolve("client::interceptors::SharedInterceptor"), - "TestParamsSetterInterceptor" to CargoDependency.smithyRuntime(runtimeConfig).withFeature("test-util") - .toType().resolve("client::test_util::interceptors::TestParamsSetterInterceptor"), + "SharedInterceptor" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::interceptors::SharedInterceptor"), + "SharedTimeSource" to CargoDependency.smithyAsync(runtimeConfig).toType().resolve("time::SharedTimeSource"), + "StaticRuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig).resolve("client::runtime_plugin::StaticRuntimePlugin"), + "StaticTimeSource" to CargoDependency.smithyAsync(runtimeConfig).toType().resolve("time::StaticTimeSource"), + "TestParamsSetterInterceptor" to testParamsSetterInterceptor(), ) + // TODO(enableNewSmithyRuntimeCleanup): Delete this once test helpers on `CustomizableOperation` have been removed + private fun testParamsSetterInterceptor(): RuntimeType = RuntimeType.forInlineFun("TestParamsSetterInterceptor", ClientRustModule.Client.customize) { + rustTemplate( + """ + mod test_params_setter_interceptor { + use aws_smithy_runtime_api::box_error::BoxError; + use aws_smithy_runtime_api::client::interceptors::context::BeforeTransmitInterceptorContextMut; + use aws_smithy_runtime_api::client::interceptors::Interceptor; + use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; + use aws_smithy_types::config_bag::ConfigBag; + use std::fmt; + + pub(super) struct TestParamsSetterInterceptor { f: F } + + impl fmt::Debug for TestParamsSetterInterceptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "TestParamsSetterInterceptor") + } + } + + impl TestParamsSetterInterceptor { + pub fn new(f: F) -> Self { Self { f } } + } + + impl Interceptor for TestParamsSetterInterceptor + where + F: Fn(&mut BeforeTransmitInterceptorContextMut<'_>, &mut ConfigBag) + Send + Sync + 'static, + { + fn modify_before_signing( + &self, + context: &mut BeforeTransmitInterceptorContextMut<'_>, + _runtime_components: &RuntimeComponents, + cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + (self.f)(context, cfg); + Ok(()) + } + } + } + use test_params_setter_interceptor::TestParamsSetterInterceptor; + """, + *codegenScope, + ) + } + override fun section(section: CustomizableOperationSection): Writable = writable { if (section is CustomizableOperationSection.CustomizableOperationImpl) { if (section.isRuntimeModeOrchestrator) { + // TODO(enableNewSmithyRuntimeCleanup): Delete these utilities rustTemplate( """ ##[doc(hidden)] @@ -49,7 +92,7 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) : #{StaticRuntimePlugin}::new() .with_runtime_components( #{RuntimeComponentsBuilder}::new("request_time_for_tests") - .with_time_source(Some(#{SharedTimeSource}::new(request_time))) + .with_time_source(Some(#{SharedTimeSource}::new(#{StaticTimeSource}::new(request_time)))) ) ) } @@ -92,7 +135,9 @@ class CustomizableOperationTestHelpers(runtimeConfig: RuntimeConfig) : ##[doc(hidden)] // This is a temporary method for testing. NEVER use it in production pub fn request_time_for_tests(mut self, request_time: ::std::time::SystemTime) -> Self { - self.operation.properties_mut().insert(#{SharedTimeSource}::new(request_time)); + self.operation.properties_mut().insert( + #{SharedTimeSource}::new(#{StaticTimeSource}::new(request_time)) + ); self } 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 ef59287b35..f236a08f49 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 @@ -216,10 +216,12 @@ class AwsInputPresignedMethod( """ // Change signature type to query params and wire up presigning config let mut props = request.properties_mut(); - props.insert(#{SharedTimeSource}::new(presigning_config.start_time())); + props.insert(#{SharedTimeSource}::new(#{StaticTimeSource}::new(presigning_config.start_time()))); """, "SharedTimeSource" to RuntimeType.smithyAsync(runtimeConfig) .resolve("time::SharedTimeSource"), + "StaticTimeSource" to RuntimeType.smithyAsync(runtimeConfig) + .resolve("time::StaticTimeSource"), ) withBlock("props.insert(", ");") { rustTemplate( diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt index 8bdfe0cdc1..a091664c1a 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt @@ -80,14 +80,14 @@ class IntegrationTestDependencies( override fun section(section: LibRsSection) = when (section) { is LibRsSection.Body -> testDependenciesOnly { if (hasTests) { - val smithyClient = CargoDependency.smithyClient(codegenContext.runtimeConfig) - .copy(features = setOf("test-util"), scope = DependencyScope.Dev) val smithyAsync = CargoDependency.smithyAsync(codegenContext.runtimeConfig) .copy(features = setOf("test-util"), scope = DependencyScope.Dev) + val smithyClient = CargoDependency.smithyClient(codegenContext.runtimeConfig) + .copy(features = setOf("test-util"), scope = DependencyScope.Dev) val smithyTypes = CargoDependency.smithyTypes(codegenContext.runtimeConfig) .copy(features = setOf("test-util"), scope = DependencyScope.Dev) - addDependency(smithyClient) addDependency(smithyAsync) + addDependency(smithyClient) addDependency(smithyTypes) addDependency(CargoDependency.smithyProtocolTestHelpers(codegenContext.runtimeConfig)) addDependency(SerdeJson) diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 8a27b1cd2c..c504dba38d 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -17,7 +17,7 @@ aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", 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" } -aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async", features = ["rt-tokio"] } +aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async", features = ["test-util", "rt-tokio"] } 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" } 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 f3cbe633af..8d93ae88fc 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 @@ -22,6 +22,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.docsOrFallback import software.amazon.smithy.rust.codegen.core.rustlang.raw import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate 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 @@ -374,9 +375,10 @@ class ServiceConfigGenerator( } writer.docs("Builder for creating a `Config`.") - writer.raw("#[derive(Clone, Default)]") - if (runtimeMode.defaultToOrchestrator) { - Attribute(Attribute.derive(RuntimeType.Debug)).render(writer) + if (runtimeMode.defaultToMiddleware) { + writer.raw("#[derive(Clone, Default)]") + } else { + writer.raw("#[derive(Clone, Debug)]") } writer.rustBlock("pub struct Builder") { if (runtimeMode.defaultToOrchestrator) { @@ -406,6 +408,22 @@ class ServiceConfigGenerator( """, ) } + } else { + // Custom implementation of Default to give the runtime components builder a name + writer.rustBlockTemplate("impl #{Default} for Builder", *codegenScope) { + writer.rustTemplate( + """ + fn default() -> Self { + Self { + config: #{Default}::default(), + runtime_components: #{RuntimeComponentsBuilder}::new("service config"), + runtime_plugins: #{Default}::default(), + } + } + """, + *codegenScope, + ) + } } writer.rustBlock("impl Builder") { diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt index 92f653bf1b..474ed7b43e 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/HttpAuthDecoratorTest.kt @@ -19,7 +19,7 @@ import software.amazon.smithy.rust.codegen.core.testutil.integrationTest class HttpAuthDecoratorTest { private fun codegenScope(runtimeConfig: RuntimeConfig): Array> = arrayOf( "TestConnection" to CargoDependency.smithyClient(runtimeConfig) - .withFeature("test-util").toType() + .toDevDependency().withFeature("test-util").toType() .resolve("test_connection::TestConnection"), "SdkBody" to RuntimeType.sdkBody(runtimeConfig), ) diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/MetadataCustomizationTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/MetadataCustomizationTest.kt index 97d88c1680..0f087d50ad 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/MetadataCustomizationTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/MetadataCustomizationTest.kt @@ -52,7 +52,7 @@ class MetadataCustomizationTest { .resolve("client::runtime_components::RuntimeComponents"), ) rustCrate.testModule { - addDependency(CargoDependency.Tokio.withFeature("test-util").toDevDependency()) + addDependency(CargoDependency.Tokio.toDevDependency().withFeature("test-util")) tokioTest("test_extract_metadata_via_customizable_operation") { rustTemplate( """ 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 index 086a211915..40f82d2965 100644 --- 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 @@ -52,7 +52,7 @@ internal class ConfigOverrideRuntimePluginGeneratorTest { "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), ) rustCrate.testModule { - addDependency(CargoDependency.Tokio.withFeature("test-util").toDevDependency()) + addDependency(CargoDependency.Tokio.toDevDependency().withFeature("test-util")) tokioTest("test_operation_overrides_endpoint_resolver") { rustTemplate( """ @@ -97,7 +97,7 @@ internal class ConfigOverrideRuntimePluginGeneratorTest { "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), ) rustCrate.testModule { - addDependency(CargoDependency.Tokio.withFeature("test-util").toDevDependency()) + addDependency(CargoDependency.Tokio.toDevDependency().withFeature("test-util")) tokioTest("test_operation_overrides_http_connection") { rustTemplate( """ diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt index de6b6f175e..1e02493212 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGeneratorTest.kt @@ -62,6 +62,7 @@ class FluentClientGeneratorTest { } """, "TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig) + .toDevDependency() .withFeature("test-util").toType() .resolve("test_connection::TestConnection"), "SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig), @@ -108,6 +109,7 @@ class FluentClientGeneratorTest { } """, "TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig) + .toDevDependency() .withFeature("test-util").toType() .resolve("test_connection::TestConnection"), "SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig), diff --git a/codegen-core/build.gradle.kts b/codegen-core/build.gradle.kts index 393eb4dfa7..556088f8a0 100644 --- a/codegen-core/build.gradle.kts +++ b/codegen-core/build.gradle.kts @@ -112,12 +112,11 @@ if (isTestingEnabled.toBoolean()) { tasks.test { useJUnitPlatform() testLogging { - events("passed", "skipped", "failed") + events("failed") exceptionFormat = TestExceptionFormat.FULL showCauses = true showExceptions = true showStackTraces = true - showStandardStreams = true } } 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 b91020fe3f..8dad31b91c 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 @@ -13,11 +13,11 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.util.dq import java.nio.file.Path -sealed class DependencyScope { - object Dev : DependencyScope() - object Compile : DependencyScope() - object CfgUnstable : DependencyScope() - object Build : DependencyScope() +enum class DependencyScope { + Build, + CfgUnstable, + Compile, + Dev, } sealed class DependencyLocation @@ -136,6 +136,8 @@ fun InlineDependency.toType() = RuntimeType(module.fullyQualifiedPath(), this) data class Feature(val name: String, val default: Boolean, val deps: List) +val DEV_ONLY_FEATURES = setOf("test-util") + /** * A dependency on an internal or external Cargo Crate */ @@ -150,6 +152,12 @@ data class CargoDependency( ) : RustDependency(name) { val key: Triple get() = Triple(name, location, scope) + init { + if (scope != DependencyScope.Dev && DEV_ONLY_FEATURES.any { features.contains(it) }) { + throw IllegalArgumentException("The `test-util` feature cannot be used outside of DependencyScope.Dev") + } + } + fun withFeature(feature: String): CargoDependency { return copy(features = features.toMutableSet().apply { add(feature) }) } @@ -205,7 +213,8 @@ data class CargoDependency( attribs.add("features = [${joinToString(",") { it.dq() }}]") } } - return "$name = { ${attribs.joinToString(",")} }" + attribs.add("scope = $scope") + return "$name = { ${attribs.joinToString(", ")} }" } fun toType(): RuntimeType { diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt index 9dece19f8b..dd17595c5f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt @@ -498,6 +498,15 @@ class RustWriter private constructor( } } + fun toml(fileName: String, debugMode: Boolean = false): RustWriter = + RustWriter( + fileName, + namespace = "ignore", + commentCharacter = "#", + printWarning = false, + debugMode = debugMode, + ) + private fun rawWriter(fileName: String, debugMode: Boolean): RustWriter = RustWriter( fileName, diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegator.kt index 157a6f2539..5ae40c3574 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegator.kt @@ -12,6 +12,7 @@ import software.amazon.smithy.codegen.core.WriterDelegator import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency +import software.amazon.smithy.rust.codegen.core.rustlang.DEV_ONLY_FEATURES import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope import software.amazon.smithy.rust.codegen.core.rustlang.Feature import software.amazon.smithy.rust.codegen.core.rustlang.InlineDependency @@ -298,7 +299,9 @@ internal fun List.mergeIdenticalTestDependencies(): List + dep.scope == DependencyScope.Dev && + DEV_ONLY_FEATURES.none { devOnly -> dep.features.contains(devOnly) } && + compileDeps.contains(dep.copy(scope = DependencyScope.Compile)) } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt index 86c76c521a..b5eca73acd 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/CargoTomlGenerator.kt @@ -43,12 +43,36 @@ typealias ManifestCustomizations = Map * Generates the crate manifest Cargo.toml file. */ class CargoTomlGenerator( - private val settings: CoreRustSettings, + private val moduleName: String, + private val moduleVersion: String, + private val moduleAuthors: List, + private val moduleDescription: String?, + private val moduleLicense: String?, + private val moduleRepository: String?, private val writer: RustWriter, - private val manifestCustomizations: ManifestCustomizations, - private val dependencies: List, - private val features: List, + private val manifestCustomizations: ManifestCustomizations = emptyMap(), + private val dependencies: List = emptyList(), + private val features: List = emptyList(), ) { + constructor( + settings: CoreRustSettings, + writer: RustWriter, + manifestCustomizations: ManifestCustomizations, + dependencies: List, + features: List, + ) : this( + settings.moduleName, + settings.moduleVersion, + settings.moduleAuthors, + settings.moduleDescription, + settings.license, + settings.moduleRepository, + writer, + manifestCustomizations, + dependencies, + features, + ) + fun render() { val cargoFeatures = features.map { it.name to it.deps }.toMutableList() if (features.isNotEmpty()) { @@ -57,13 +81,13 @@ class CargoTomlGenerator( val cargoToml = mapOf( "package" to listOfNotNull( - "name" to settings.moduleName, - "version" to settings.moduleVersion, - "authors" to settings.moduleAuthors, - settings.moduleDescription?.let { "description" to it }, + "name" to moduleName, + "version" to moduleVersion, + "authors" to moduleAuthors, + moduleDescription?.let { "description" to it }, "edition" to "2021", - "license" to settings.license, - "repository" to settings.moduleRepository, + "license" to moduleLicense, + "repository" to moduleRepository, "metadata" to listOfNotNull( "smithy" to listOfNotNull( "codegen-version" to Version.fullVersion(), 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 2bf386a519..8f8e6ff8b5 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 @@ -32,8 +32,10 @@ import software.amazon.smithy.rust.codegen.core.smithy.ModuleDocProvider import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider +import software.amazon.smithy.rust.codegen.core.smithy.generators.CargoTomlGenerator +import software.amazon.smithy.rust.codegen.core.smithy.mergeDependencyFeatures +import software.amazon.smithy.rust.codegen.core.smithy.mergeIdenticalTestDependencies import software.amazon.smithy.rust.codegen.core.util.CommandError -import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.letIf import software.amazon.smithy.rust.codegen.core.util.orNullIfEmpty import software.amazon.smithy.rust.codegen.core.util.runCommand @@ -369,14 +371,19 @@ fun RustWriter.compileAndTest( clippy: Boolean = false, expectFailure: Boolean = false, ): String { - val deps = this.dependencies.map { RustDependency.fromSymbolDependency(it) }.filterIsInstance() + val deps = this.dependencies + .map { RustDependency.fromSymbolDependency(it) } + .filterIsInstance() + .distinct() + .mergeDependencyFeatures() + .mergeIdenticalTestDependencies() val module = if (this.namespace.contains("::")) { this.namespace.split("::")[1] } else { "lib" } val tempDir = this.toString() - .intoCrate(deps.toSet(), module = module, main = main, strict = clippy) + .intoCrate(deps, module = module, main = main, strict = clippy) val mainRs = tempDir.resolve("src/main.rs") val testModule = tempDir.resolve("src/$module.rs") try { @@ -398,23 +405,25 @@ fun RustWriter.compileAndTest( } private fun String.intoCrate( - deps: Set, + deps: List, module: String? = null, main: String = "", strict: Boolean = false, ): File { this.shouldParseAsRust() val tempDir = TestWorkspace.subproject() - val cargoToml = """ - [package] - name = ${tempDir.nameWithoutExtension.dq()} - version = "0.0.1" - authors = ["rcoh@amazon.com"] - edition = "2021" - - [dependencies] - ${deps.joinToString("\n") { it.toString() }} - """.trimIndent() + val cargoToml = RustWriter.toml("Cargo.toml").apply { + CargoTomlGenerator( + moduleName = tempDir.nameWithoutExtension, + moduleVersion = "0.0.1", + moduleAuthors = listOf("Testy McTesterson"), + moduleDescription = null, + moduleLicense = null, + moduleRepository = null, + writer = this, + dependencies = deps, + ).render() + }.toString() tempDir.resolve("Cargo.toml").writeText(cargoToml) tempDir.resolve("src").mkdirs() val mainRs = tempDir.resolve("src/main.rs") diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependencyTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependencyTest.kt new file mode 100644 index 0000000000..9d18c4725e --- /dev/null +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependencyTest.kt @@ -0,0 +1,48 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.core.rustlang + +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +class CargoDependencyTest { + @Test + fun `it should not allow a dependency with test-util in non-dev scopes`() { + // OK + CargoDependency( + name = "test", + location = CratesIo("1.0"), + features = setOf("foo", "test-util", "bar"), + scope = DependencyScope.Dev, + ) + + // OK + CargoDependency( + name = "test", + location = CratesIo("1.0"), + features = setOf("foo", "bar"), + scope = DependencyScope.Dev, + ).toDevDependency().withFeature("test-util") + + assertThrows { + CargoDependency( + name = "test", + location = CratesIo("1.0"), + features = setOf("foo", "test-util", "bar"), + scope = DependencyScope.Compile, + ) + } + + assertThrows { + CargoDependency( + name = "test", + location = CratesIo("1.0"), + features = setOf("foo", "bar"), + scope = DependencyScope.Compile, + ).withFeature("test-util") + } + } +} diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegatorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegatorTest.kt index c9407372d7..6ed2fc5ed3 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegatorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CodegenDelegatorTest.kt @@ -6,6 +6,7 @@ package software.amazon.smithy.rust.codegen.core.smithy import io.kotest.matchers.shouldBe +import org.junit.jupiter.api.RepeatedTest import org.junit.jupiter.api.Test import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.CratesIo @@ -13,7 +14,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope.Compile import software.amazon.smithy.rust.codegen.core.rustlang.DependencyScope.Dev class CodegenDelegatorTest { - @Test + @RepeatedTest(10) // Test it several times since the shuffle adds in some randomness fun testMergeDependencyFeatures() { val merged = listOf( @@ -36,6 +37,22 @@ class CodegenDelegatorTest { ) } + @RepeatedTest(10) // Test it several times since the shuffle adds in some randomness + fun testMergeDependencyFeaturesDontMergeDevOnlyFeatures() { + val merged = listOf( + CargoDependency("A", CratesIo("1"), Compile, features = setOf("a")), + CargoDependency("A", CratesIo("1"), Compile, features = setOf("b")), + CargoDependency("A", CratesIo("1"), Dev, features = setOf("c")), + CargoDependency("A", CratesIo("1"), Dev, features = setOf("test-util")), + ).shuffled().mergeDependencyFeatures() + .sortedBy { it.scope } + + merged shouldBe setOf( + CargoDependency("A", CratesIo("1"), Compile, features = setOf("a", "b")), + CargoDependency("A", CratesIo("1"), Dev, features = setOf("c", "test-util")), + ) + } + @Test fun testMergeIdenticalFeatures() { val merged = listOf( @@ -43,11 +60,15 @@ class CodegenDelegatorTest { CargoDependency("A", CratesIo("1"), Dev), CargoDependency("B", CratesIo("1"), Compile), CargoDependency("B", CratesIo("1"), Dev, features = setOf("a", "b")), + CargoDependency("C", CratesIo("1"), Compile), + CargoDependency("C", CratesIo("1"), Dev, features = setOf("test-util")), ).mergeIdenticalTestDependencies() merged shouldBe setOf( CargoDependency("A", CratesIo("1"), Compile), CargoDependency("B", CratesIo("1"), Compile), CargoDependency("B", CratesIo("1"), Dev, features = setOf("a", "b")), + CargoDependency("C", CratesIo("1"), Compile), + CargoDependency("C", CratesIo("1"), Dev, features = setOf("test-util")), ) } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs index 9b24628997..a00fe57671 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs @@ -134,7 +134,7 @@ macro_rules! declare_runtime_components { $($field_name: runtime_component_field_type!($outer_type $inner_type $($option)?),)+ } - #[derive(Clone, Debug, Default)] + #[derive(Clone, Debug)] pub struct $builder_name { builder_name: &'static str, $($field_name: $outer_type>,)+ @@ -200,8 +200,8 @@ declare_runtime_components! { impl RuntimeComponents { /// Returns a builder for runtime components. - pub fn builder() -> RuntimeComponentsBuilder { - Default::default() + pub fn builder(name: &'static str) -> RuntimeComponentsBuilder { + RuntimeComponentsBuilder::new(name) } /// Returns the auth option resolver. diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index 803310221f..38b8852acb 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -162,7 +162,7 @@ fn apply_configuration( continue_on_err!([ctx] => Interceptors::new(operation_rc_builder.interceptors()).read_before_execution(true, ctx, cfg)); // The order below is important. Client interceptors must run before operation interceptors. - Ok(RuntimeComponents::builder() + Ok(RuntimeComponents::builder("merged orchestrator components") .merge_from(&client_rc_builder) .merge_from(&operation_rc_builder) .build()?) 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 de7aef4ac5..30adb4f6cb 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/test_util.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/test_util.rs @@ -4,5 +4,4 @@ */ pub mod deserializer; -pub mod interceptors; pub mod serializer; diff --git a/rust-runtime/aws-smithy-runtime/src/client/test_util/interceptors.rs b/rust-runtime/aws-smithy-runtime/src/client/test_util/interceptors.rs deleted file mode 100644 index 957f04bab3..0000000000 --- a/rust-runtime/aws-smithy-runtime/src/client/test_util/interceptors.rs +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -// TODO(enableNewSmithyRuntimeCleanup): Delete this file once test helpers on `CustomizableOperation` have been removed - -use aws_smithy_runtime_api::box_error::BoxError; -use aws_smithy_runtime_api::client::interceptors::context::BeforeTransmitInterceptorContextMut; -use aws_smithy_runtime_api::client::interceptors::Interceptor; -use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; -use aws_smithy_types::config_bag::ConfigBag; -use std::fmt; - -pub struct TestParamsSetterInterceptor { - f: F, -} - -impl fmt::Debug for TestParamsSetterInterceptor { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "TestParamsSetterInterceptor") - } -} - -impl TestParamsSetterInterceptor { - pub fn new(f: F) -> Self { - Self { f } - } -} - -impl Interceptor for TestParamsSetterInterceptor -where - F: Fn(&mut BeforeTransmitInterceptorContextMut<'_>, &mut ConfigBag) + Send + Sync + 'static, -{ - fn modify_before_signing( - &self, - context: &mut BeforeTransmitInterceptorContextMut<'_>, - _runtime_components: &RuntimeComponents, - cfg: &mut ConfigBag, - ) -> Result<(), BoxError> { - (self.f)(context, cfg); - - Ok(()) - } -}