Skip to content

Commit

Permalink
Fix KMS tests by fixing Send bound on fluent send method (#2695)
Browse files Browse the repository at this point in the history
## Motivation and Context
This PR fixes the KMS tests when the generating code in orchestrator
mode by ensuring the `Future` returned by `send()` implements `Send`,
and also adds a codegen unit test for this so that it's checked at the
generic client level.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
jdisanti authored and david-perez committed May 22, 2023
1 parent 3dad4e3 commit 9bf3f04
Show file tree
Hide file tree
Showing 11 changed files with 155 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ async fn sra_test() {
conn.full_validate(MediaType::Xml).await.expect("failed")
}

#[derive(Debug)]
struct FixupPlugin {
timestamp: SystemTime,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class ServiceRuntimePluginGenerator(
fun render(writer: RustWriter, customizations: List<ServiceRuntimePluginCustomization>) {
writer.rustTemplate(
"""
##[derive(Debug)]
pub(crate) struct ServiceRuntimePlugin {
handle: std::sync::Arc<crate::client::Handle>,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,13 @@ class FluentClientGenerator(
"""
##[doc(hidden)]
pub async fn send_orchestrator(self) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> {
self.send_orchestrator_with_plugin(Option::<Box<dyn #{RuntimePlugin}>>::None).await
self.send_orchestrator_with_plugin(Option::<Box<dyn #{RuntimePlugin} + Send + Sync>>::None).await
}
##[doc(hidden)]
// TODO(enableNewSmithyRuntime): Delete when unused
/// Equivalent to [`Self::send_orchestrator`] but adds a final runtime plugin to shim missing behavior
pub async fn send_orchestrator_with_plugin(self, final_plugin: Option<impl #{RuntimePlugin} + 'static>) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> {
pub async fn send_orchestrator_with_plugin(self, final_plugin: Option<impl #{RuntimePlugin} + Send + Sync + 'static>) -> std::result::Result<#{OperationOutput}, #{SdkError}<#{OperationError}, #{HttpResponse}>> {
let mut runtime_plugins = #{RuntimePlugins}::new()
.with_client_plugin(crate::config::ServiceRuntimePlugin::new(self.handle.clone()));
if let Some(config_override) = self.config_override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,16 @@
package software.amazon.smithy.rust.codegen.client.smithy.customizations

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.CargoDependency
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
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.testutil.IntegrationTestParams
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest

private fun additionalSettings(): ObjectNode = ObjectNode.objectNodeBuilder()
.withMember(
"codegen",
ObjectNode.objectNodeBuilder()
.withMember("enableNewSmithyRuntime", StringNode.from("orchestrator")).build(),
)
.build()

class HttpAuthDecoratorTest {
private fun codegenScope(runtimeConfig: RuntimeConfig): Array<Pair<String, Any>> = arrayOf(
"TestConnection" to CargoDependency.smithyClient(runtimeConfig)
Expand All @@ -38,7 +28,7 @@ class HttpAuthDecoratorTest {
fun multipleAuthSchemesSchemeSelection() {
clientIntegrationTest(
TestModels.allSchemes,
IntegrationTestParams(additionalSettings = additionalSettings()),
TestCodegenSettings.orchestratorModeTestParams,
) { codegenContext, rustCrate ->
rustCrate.integrationTest("tests") {
val moduleName = codegenContext.moduleUseName()
Expand Down Expand Up @@ -117,7 +107,7 @@ class HttpAuthDecoratorTest {
fun apiKeyInQueryString() {
clientIntegrationTest(
TestModels.apiKeyInQueryString,
IntegrationTestParams(additionalSettings = additionalSettings()),
TestCodegenSettings.orchestratorModeTestParams,
) { codegenContext, rustCrate ->
rustCrate.integrationTest("api_key_applied_to_query_string") {
val moduleName = codegenContext.moduleUseName()
Expand Down Expand Up @@ -162,7 +152,7 @@ class HttpAuthDecoratorTest {
fun apiKeyInHeaders() {
clientIntegrationTest(
TestModels.apiKeyInHeaders,
IntegrationTestParams(additionalSettings = additionalSettings()),
TestCodegenSettings.orchestratorModeTestParams,
) { codegenContext, rustCrate ->
rustCrate.integrationTest("api_key_applied_to_headers") {
val moduleName = codegenContext.moduleUseName()
Expand Down Expand Up @@ -208,7 +198,7 @@ class HttpAuthDecoratorTest {
fun basicAuth() {
clientIntegrationTest(
TestModels.basicAuth,
IntegrationTestParams(additionalSettings = additionalSettings()),
TestCodegenSettings.orchestratorModeTestParams,
) { codegenContext, rustCrate ->
rustCrate.integrationTest("basic_auth") {
val moduleName = codegenContext.moduleUseName()
Expand Down Expand Up @@ -254,7 +244,7 @@ class HttpAuthDecoratorTest {
fun bearerAuth() {
clientIntegrationTest(
TestModels.bearerAuth,
IntegrationTestParams(additionalSettings = additionalSettings()),
TestCodegenSettings.orchestratorModeTestParams,
) { codegenContext, rustCrate ->
rustCrate.integrationTest("bearer_auth") {
val moduleName = codegenContext.moduleUseName()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* 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.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.rust
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.rustlang.writable
import software.amazon.smithy.rust.codegen.core.smithy.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 FluentClientGeneratorTest {
val model = """
namespace com.example
use aws.protocols#awsJson1_0
@awsJson1_0
service HelloService {
operations: [SayHello],
version: "1"
}
operation SayHello { input: TestInput }
structure TestInput {}
""".asSmithyModel()

@Test
fun `send() future implements Send`() {
val test: (ClientCodegenContext, RustCrate) -> Unit = { codegenContext, rustCrate ->
rustCrate.integrationTest("send_future_is_send") {
val moduleName = codegenContext.moduleUseName()
rustTemplate(
"""
fn check_send<T: Send>(_: T) {}
##[test]
fn test() {
let connector = #{TestConnection}::<#{SdkBody}>::new(Vec::new());
let config = $moduleName::Config::builder()
.endpoint_resolver("http://localhost:1234")
#{set_http_connector}
.build();
let smithy_client = aws_smithy_client::Builder::new()
.connector(connector.clone())
.middleware_fn(|r| r)
.build_dyn();
let client = $moduleName::Client::with_config(smithy_client, config);
check_send(client.say_hello().send());
}
""",
"TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig)
.withFeature("test-util").toType()
.resolve("test_connection::TestConnection"),
"SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig),
"set_http_connector" to writable {
if (codegenContext.smithyRuntimeMode.generateOrchestrator) {
rust(".http_connector(connector.clone())")
}
},
)
}
}
clientIntegrationTest(model, TestCodegenSettings.middlewareModeTestParams, test = test)
clientIntegrationTest(
model,
TestCodegenSettings.orchestratorModeTestParams,
test = test,
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rust.codegen.client.testutil

import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.model.node.StringNode
import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams

object TestCodegenSettings {
// TODO(enableNewSmithyRuntime): Delete this when removing `enableNewSmithyRuntime` feature gate
fun middlewareMode(): ObjectNode = ObjectNode.objectNodeBuilder()
.withMember(
"codegen",
ObjectNode.objectNodeBuilder()
.withMember("enableNewSmithyRuntime", StringNode.from("middleware")).build(),
)
.build()

// TODO(enableNewSmithyRuntime): Delete this when removing `enableNewSmithyRuntime` feature gate
fun orchestratorMode(): ObjectNode = ObjectNode.objectNodeBuilder()
.withMember(
"codegen",
ObjectNode.objectNodeBuilder()
.withMember("enableNewSmithyRuntime", StringNode.from("orchestrator")).build(),
)
.build()

// TODO(enableNewSmithyRuntime): Delete this when removing `enableNewSmithyRuntime` feature gate
val middlewareModeTestParams get(): IntegrationTestParams =
IntegrationTestParams(additionalSettings = middlewareMode())

// TODO(enableNewSmithyRuntime): Delete this when removing `enableNewSmithyRuntime` feature gate
val orchestratorModeTestParams get(): IntegrationTestParams =
IntegrationTestParams(additionalSettings = orchestratorMode())
}
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-runtime-api/src/client/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl AuthOptionResolver for Box<dyn AuthOptionResolver> {
struct HttpAuthSchemesInner {
schemes: Vec<(AuthSchemeId, Box<dyn HttpAuthScheme>)>,
}
#[derive(Debug)]
#[derive(Clone, Debug)]
pub struct HttpAuthSchemes {
inner: Arc<HttpAuthSchemesInner>,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::time::SystemTime;
pub type HttpRequest = http::Request<SdkBody>;
pub type HttpResponse = http::Response<SdkBody>;
pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
pub type BoxFuture<T> = Pin<Box<dyn StdFuture<Output = Result<T, BoxError>>>>;
pub type BoxFuture<T> = Pin<Box<dyn StdFuture<Output = Result<T, BoxError>> + Send>>;
pub type Future<T> = NowOrLater<Result<T, BoxError>, BoxFuture<T>>;

pub trait RequestSerializer: Send + Sync + fmt::Debug {
Expand Down
23 changes: 16 additions & 7 deletions rust-runtime/aws-smithy-runtime-api/src/client/runtime_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@

use crate::client::interceptors::Interceptors;
use crate::config_bag::ConfigBag;
use std::fmt::Debug;

pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
pub type BoxError = Box<dyn std::error::Error + Send + Sync>;
pub type BoxRuntimePlugin = Box<dyn RuntimePlugin + Send + Sync>;

pub trait RuntimePlugin {
pub trait RuntimePlugin: Debug {
fn configure(
&self,
cfg: &mut ConfigBag,
interceptors: &mut Interceptors,
) -> Result<(), BoxError>;
}

impl RuntimePlugin for Box<dyn RuntimePlugin> {
impl RuntimePlugin for BoxRuntimePlugin {
fn configure(
&self,
cfg: &mut ConfigBag,
Expand All @@ -28,21 +30,27 @@ impl RuntimePlugin for Box<dyn RuntimePlugin> {

#[derive(Default)]
pub struct RuntimePlugins {
client_plugins: Vec<Box<dyn RuntimePlugin>>,
operation_plugins: Vec<Box<dyn RuntimePlugin>>,
client_plugins: Vec<BoxRuntimePlugin>,
operation_plugins: Vec<BoxRuntimePlugin>,
}

impl RuntimePlugins {
pub fn new() -> Self {
Default::default()
}

pub fn with_client_plugin(mut self, plugin: impl RuntimePlugin + 'static) -> Self {
pub fn with_client_plugin(
mut self,
plugin: impl RuntimePlugin + Send + Sync + 'static,
) -> Self {
self.client_plugins.push(Box::new(plugin));
self
}

pub fn with_operation_plugin(mut self, plugin: impl RuntimePlugin + 'static) -> Self {
pub fn with_operation_plugin(
mut self,
plugin: impl RuntimePlugin + Send + Sync + 'static,
) -> Self {
self.operation_plugins.push(Box::new(plugin));
self
}
Expand Down Expand Up @@ -78,6 +86,7 @@ mod tests {
use crate::client::interceptors::Interceptors;
use crate::config_bag::ConfigBag;

#[derive(Debug)]
struct SomeStruct;

impl RuntimePlugin for SomeStruct {
Expand Down
2 changes: 2 additions & 0 deletions rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ mod tests {
)
}

#[derive(Debug)]
struct TestOperationRuntimePlugin;

impl RuntimePlugin for TestOperationRuntimePlugin {
Expand Down Expand Up @@ -310,6 +311,7 @@ mod tests {
}
}

#[derive(Debug)]
struct FailingInterceptorsOperationRuntimePlugin;

impl RuntimePlugin for FailingInterceptorsOperationRuntimePlugin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ const ANONYMOUS_AUTH_SCHEME_ID: AuthSchemeId = AuthSchemeId::new("anonymous");
/// **The above components will replace any existing ones!** As such, don't use this plugin unless:
/// - You only need to make anonymous requests, such as when interacting with [Open Data](https://aws.amazon.com/opendata/).
/// - You're writing orchestrator tests and don't care about authentication.
#[non_exhaustive]
#[derive(Debug, Default)]
pub struct AnonymousAuthRuntimePlugin;

impl AnonymousAuthRuntimePlugin {
pub fn new() -> Self {
Self
}
}

impl RuntimePlugin for AnonymousAuthRuntimePlugin {
fn configure(
&self,
Expand Down

0 comments on commit 9bf3f04

Please sign in to comment.