diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 6c92ef896b2e3d07089c88c441141e2b5e4569fa..e538133e782d0bfd10333f889ca00a1a6e2b2089 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -66,3 +66,21 @@ message = "Allow `no_credentials` to be used with all S3 operations." references = ["smithy-rs#2955", "aws-sdk-rust#878"] meta = { "breaking" = false, "tada" = false, "bug" = true } author = "jdisanti" + +[[aws-sdk-rust]] +message = "`CustomizableOperation`, created as a result of calling the `.customize` method on a fluent builder, ceased to be `Send` and `Sync` in the previous releases. It is now `Send` and `Sync` again." +references = ["smithy-rs#2944", "smithy-rs#2951"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" + +[[smithy-rs]] +message = "`CustomizableOperation`, created as a result of calling the `.customize` method on a fluent builder, ceased to be `Send` and `Sync` in the previous releases. It is now `Send` and `Sync` again." +references = ["smithy-rs#2944", "smithy-rs#2951"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } +author = "ysaito1001" + +[[smithy-rs]] +message = "Generate a region setter when a model uses SigV4." +references = ["smithy-rs#2960"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } +author = "jdisanti" diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt index 804e4ec3c324066deeb1e1fbea300cc1c12e66ff..a0d47cc4d154f4be7e581aa5182ec67ff5140898 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt @@ -5,6 +5,8 @@ package software.amazon.smithy.rustsdk +import software.amazon.smithy.aws.traits.auth.SigV4Trait +import software.amazon.smithy.model.knowledge.ServiceIndex import software.amazon.smithy.model.node.Node import software.amazon.smithy.rulesengine.language.syntax.parameters.Builtins import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameter @@ -79,7 +81,11 @@ class RegionDecorator : ClientCodegenDecorator { override val name: String = "Region" override val order: Byte = 0 - private fun usesRegion(codegenContext: ClientCodegenContext) = codegenContext.getBuiltIn(Builtins.REGION) != null + // Services that have an endpoint ruleset that references the SDK::Region built in, or + // that use SigV4, both need a configurable region. + private fun usesRegion(codegenContext: ClientCodegenContext) = + codegenContext.getBuiltIn(Builtins.REGION) != null || ServiceIndex.of(codegenContext.model) + .getEffectiveAuthSchemes(codegenContext.serviceShape).containsKey(SigV4Trait.ID) override fun configCustomizations( codegenContext: ClientCodegenContext, diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/RegionDecoratorTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/RegionDecoratorTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..cc003fc08646459b3c622ff25868e169cfccf5ab --- /dev/null +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/RegionDecoratorTest.kt @@ -0,0 +1,102 @@ +/* + * 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.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import kotlin.io.path.readText + +class RegionDecoratorTest { + private val modelWithoutRegionParamOrSigV4AuthScheme = """ + namespace test + + use aws.api#service + use aws.protocols#awsJson1_0 + use smithy.rules#endpointRuleSet + + @awsJson1_0 + @endpointRuleSet({ + "version": "1.0", + "rules": [{ "type": "endpoint", "conditions": [], "endpoint": { "url": "https://example.com" } }], + "parameters": {} + }) + @service(sdkId: "dontcare") + service TestService { version: "2023-01-01", operations: [SomeOperation] } + structure SomeOutput { something: String } + operation SomeOperation { output: SomeOutput } + """.asSmithyModel() + + private val modelWithRegionParam = """ + namespace test + + use aws.api#service + use aws.protocols#awsJson1_0 + use smithy.rules#endpointRuleSet + + @awsJson1_0 + @endpointRuleSet({ + "version": "1.0", + "rules": [{ "type": "endpoint", "conditions": [], "endpoint": { "url": "https://example.com" } }], + "parameters": { + "Region": { "required": false, "type": "String", "builtIn": "AWS::Region" }, + } + }) + @service(sdkId: "dontcare") + service TestService { version: "2023-01-01", operations: [SomeOperation] } + structure SomeOutput { something: String } + operation SomeOperation { output: SomeOutput } + """.asSmithyModel() + + private val modelWithSigV4AuthScheme = """ + namespace test + + use aws.auth#sigv4 + use aws.api#service + use aws.protocols#awsJson1_0 + use smithy.rules#endpointRuleSet + + @auth([sigv4]) + @sigv4(name: "dontcare") + @awsJson1_0 + @endpointRuleSet({ + "version": "1.0", + "rules": [{ "type": "endpoint", "conditions": [], "endpoint": { "url": "https://example.com" } }], + "parameters": {} + }) + @service(sdkId: "dontcare") + service TestService { version: "2023-01-01", operations: [SomeOperation] } + structure SomeOutput { something: String } + operation SomeOperation { output: SomeOutput } + """.asSmithyModel() + + @Test + fun `models without region built-in params or SigV4 should not have configurable regions`() { + val path = awsSdkIntegrationTest(modelWithoutRegionParamOrSigV4AuthScheme) { _, _ -> + // it should generate and compile successfully + } + val configContents = path.resolve("src/config.rs").readText() + assertFalse(configContents.contains("fn set_region(")) + } + + @Test + fun `models with region built-in params should have configurable regions`() { + val path = awsSdkIntegrationTest(modelWithRegionParam) { _, _ -> + // it should generate and compile successfully + } + val configContents = path.resolve("src/config.rs").readText() + assertTrue(configContents.contains("fn set_region(")) + } + + @Test + fun `models with SigV4 should have configurable regions`() { + val path = awsSdkIntegrationTest(modelWithSigV4AuthScheme) { _, _ -> + // it should generate and compile successfully + } + val configContents = path.resolve("src/config.rs").readText() + assertTrue(configContents.contains("fn set_region(")) + } +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt index bfac81dd160b156b451eb24bb3273251923897e0..c6f9dd6b0a70e31f00f754de79029d60a9f078fc 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGenerator.kt @@ -44,6 +44,7 @@ class CustomizableOperationGenerator( .resolve("client::interceptors::MapRequestInterceptor"), "MutateRequestInterceptor" to RuntimeType.smithyRuntime(runtimeConfig) .resolve("client::interceptors::MutateRequestInterceptor"), + "PhantomData" to RuntimeType.Phantom, "RuntimePlugin" to RuntimeType.runtimePlugin(runtimeConfig), "SharedRuntimePlugin" to RuntimeType.sharedRuntimePlugin(runtimeConfig), "SendResult" to ClientRustModule.Client.customize.toType() @@ -61,14 +62,28 @@ class CustomizableOperationGenerator( rustTemplate( """ /// `CustomizableOperation` allows for configuring a single operation invocation before it is sent. - pub struct CustomizableOperation { - pub(crate) customizable_send: #{Box}>, - pub(crate) config_override: #{Option}, - pub(crate) interceptors: Vec<#{SharedInterceptor}>, - pub(crate) runtime_plugins: Vec<#{SharedRuntimePlugin}>, + pub struct CustomizableOperation { + customizable_send: B, + config_override: #{Option}, + interceptors: Vec<#{SharedInterceptor}>, + runtime_plugins: Vec<#{SharedRuntimePlugin}>, + _output: #{PhantomData}, + _error: #{PhantomData}, } - impl CustomizableOperation { + impl CustomizableOperation { + /// Creates a new `CustomizableOperation` from `customizable_send`. + pub(crate) fn new(customizable_send: B) -> Self { + Self { + customizable_send, + config_override: #{None}, + interceptors: vec![], + runtime_plugins: vec![], + _output: #{PhantomData}, + _error: #{PhantomData} + } + } + /// Adds an [`Interceptor`](#{Interceptor}) that runs at specific stages of the request execution pipeline. /// /// Note that interceptors can also be added to `CustomizableOperation` by `config_override`, @@ -142,6 +157,7 @@ class CustomizableOperationGenerator( ) -> #{SendResult} where E: std::error::Error + #{Send} + #{Sync} + 'static, + B: #{CustomizableSend}, { let mut config_override = self.config_override.unwrap_or_default(); self.interceptors.into_iter().for_each(|interceptor| { @@ -151,7 +167,7 @@ class CustomizableOperationGenerator( config_override.push_runtime_plugin(plugin); }); - (self.customizable_send)(config_override).await + self.customizable_send.send(config_override).await } #{additional_methods} @@ -182,14 +198,12 @@ class CustomizableOperationGenerator( >, >; - pub trait CustomizableSend: - #{FnOnce}(crate::config::Builder) -> BoxFuture> - {} - - impl CustomizableSend for F - where - F: #{FnOnce}(crate::config::Builder) -> BoxFuture> - {} + pub trait CustomizableSend: #{Send} + #{Sync} { + // Takes an owned `self` as the implementation will internally call methods that take `self`. + // If it took `&self`, that would make this trait object safe, but some implementing types do not + // derive `Clone`, unable to yield `self` from `&self`. + fn send(self, config_override: crate::config::Builder) -> BoxFuture>; + } """, *preludeScope, "HttpResponse" to RuntimeType.smithyRuntimeApi(runtimeConfig) 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 3684ae125397e2e54f576ecf67baeb1b13294be5..b2927dca8e550908db41c3c469e96cb8d8814c4b 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 @@ -299,6 +299,33 @@ class FluentClientGenerator( rustTemplate("config_override: #{Option},", *preludeScope) } + rustTemplate( + """ + impl + crate::client::customize::internal::CustomizableSend< + #{OperationOutput}, + #{OperationError}, + > for $builderName + { + fn send( + self, + config_override: crate::config::Builder, + ) -> crate::client::customize::internal::BoxFuture< + crate::client::customize::internal::SendResult< + #{OperationOutput}, + #{OperationError}, + >, + > { + #{Box}::pin(async move { self.config_override(config_override).send().await }) + } + } + """, + *preludeScope, + "OperationError" to errorType, + "OperationOutput" to outputType, + "SdkError" to RuntimeType.sdkError(runtimeConfig), + ) + rustBlockTemplate( "impl $builderName", "client" to RuntimeType.smithyClient(runtimeConfig), @@ -369,22 +396,12 @@ class FluentClientGenerator( #{CustomizableOperation}< #{OperationOutput}, #{OperationError}, + Self, >, #{SdkError}<#{OperationError}>, > { - #{Ok}(#{CustomizableOperation} { - customizable_send: #{Box}::new(move |config_override| { - #{Box}::pin(async { - self.config_override(config_override) - .send() - .await - }) - }), - config_override: None, - interceptors: vec![], - runtime_plugins: vec![], - }) + #{Ok}(#{CustomizableOperation}::new(self)) } """, *orchestratorScope, diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGeneratorTest.kt new file mode 100644 index 0000000000000000000000000000000000000000..8fb6bb888f36cdf445d5a5b6e92ab87d3d47830e --- /dev/null +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/CustomizableOperationGeneratorTest.kt @@ -0,0 +1,66 @@ +/* + * 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.client + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +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.rustTemplate +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.RustCrate +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.integrationTest + +class CustomizableOperationGeneratorTest { + 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 `CustomizableOperation is send and sync`() { + val test: (ClientCodegenContext, RustCrate) -> Unit = { codegenContext, rustCrate -> + rustCrate.integrationTest("customizable_operation_is_send_and_sync") { + val moduleName = codegenContext.moduleUseName() + rustTemplate( + """ + fn check_send_and_sync(_: T) {} + + ##[test] + fn test() { + let connector = #{TestConnection}::<#{SdkBody}>::new(Vec::new()); + let config = $moduleName::Config::builder() + .http_connector(connector.clone()) + .endpoint_resolver("http://localhost:1234") + .build(); + let client = $moduleName::Client::from_conf(config); + check_send_and_sync(client.say_hello().customize()); + } + """, + "TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig) + .toDevDependency() + .withFeature("test-util").toType() + .resolve("test_connection::TestConnection"), + "SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig), + ) + } + } + clientIntegrationTest(model, test = test) + } +}