Skip to content

Commit

Permalink
Merge branch 'main' into RFC30/test2
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas-k-cameron authored Jul 6, 2023
2 parents 8d78bd4 + ddba460 commit 30e6db0
Show file tree
Hide file tree
Showing 101 changed files with 2,461 additions and 592 deletions.
40 changes: 38 additions & 2 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -371,6 +372,8 @@ where
PrintService { inner, name: Op::ID.name() }
}
}
impl HttpMarker for PrintPlugin { }
```
Alternatively, using the new `ServiceShape`, implemented on `Ser`:
Expand All @@ -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:
Expand All @@ -413,6 +421,34 @@ let plugin = /* some plugin */;
let layer = LayerPlugin::new::<SomeProtocol, SomeOperation>(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
Expand Down Expand Up @@ -495,8 +531,8 @@ let scoped_plugin = Scoped::new::<SomeScope>(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]]
Expand Down
2 changes: 2 additions & 0 deletions aws/rust-runtime/aws-config/src/test_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<W> {
buf: Arc<Mutex<Vec<u8>>>,
quiet: bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -25,10 +26,12 @@ class RetryClassifierDecorator : ClientCodegenDecorator {
operation: OperationShape,
baseCustomizations: List<OperationCustomization>,
): List<OperationCustomization> =
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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -58,6 +62,9 @@ fun awsSdkIntegrationTest(
"codegen",
ObjectNode.builder()
.withMember("includeFluentClient", false)
.letIf(defaultToOrchestrator) {
it.withMember("enableNewSmithyRuntime", StringNode.from("orchestrator"))
}
.build(),
).build(),
),
Expand Down
Loading

0 comments on commit 30e6db0

Please sign in to comment.