Skip to content

Commit

Permalink
Avoid exposing Arced ResolveEndpoint in public API (#2758)
Browse files Browse the repository at this point in the history
## Motivation and Context
Hides `Arc<dyn ResolveEndpoint>` from public API.

## Description
This PR replaces the occurrences of `Arc<dyn ResolveEndpoint>` with
`SharedEndpointResolver` to not expose bare `Arc`s in the public API.

## Testing
- [x] Passed tests in CI

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [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._

---------

Co-authored-by: Yuki Saito <[email protected]>
  • Loading branch information
ysaito1001 and ysaito1001 authored Jun 9, 2023
1 parent 5eb0927 commit ec45767
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 10 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,9 @@ filter_by_operation_id(plugin, |id| id.absolute() != "namespace#name");
author = "82marbag"
references = ["smithy-rs#2678"]
meta = { "breaking" = true, "tada" = false, "bug" = false }

[[smithy-rs]]
message = "The occurrences of `Arc<dyn ResolveEndpoint>` have now been replaced with `SharedEndpointResolver` in public APIs."
references = ["smithy-rs#2758"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
author = "ysaito1001"
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class TimestreamDecorator : ClientCodegenDecorator {
},
)
}

override fun extras(codegenContext: ClientCodegenContext, rustCrate: RustCrate) {
val endpointDiscovery = InlineAwsDependency.forRustFile(
"endpoint_discovery",
Expand Down Expand Up @@ -87,7 +88,7 @@ class TimestreamDecorator : ClientCodegenDecorator {
time
)
.await?;
new_conf.endpoint_resolver = ::std::sync::Arc::new(resolver);
new_conf.endpoint_resolver = #{SharedEndpointResolver}::new(resolver);
Ok((Self::from_conf(new_conf), reloader))
}
}
Expand All @@ -96,6 +97,8 @@ class TimestreamDecorator : ClientCodegenDecorator {
"endpoint_discovery" to endpointDiscovery.toType(),
"SystemTime" to RuntimeType.std.resolve("time::SystemTime"),
"Duration" to RuntimeType.std.resolve("time::Duration"),
"SharedEndpointResolver" to RuntimeType.smithyHttp(codegenContext.runtimeConfig)
.resolve("endpoint::SharedEndpointResolver"),
"SystemTimeSource" to RuntimeType.smithyAsync(codegenContext.runtimeConfig)
.resolve("time::SystemTimeSource"),
*Types(codegenContext.runtimeConfig).toArray(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope

/**
* Customization which injects an Endpoints 2.0 Endpoint Resolver into the service config struct
Expand All @@ -28,22 +29,25 @@ internal class EndpointConfigCustomization(

override fun section(section: ServiceConfig): Writable {
return writable {
val sharedEndpointResolver = "#{SharedEndpointResolver}<#{Params}>"
val resolverTrait = "#{SmithyResolver}<#{Params}>"
val codegenScope = arrayOf(
*preludeScope,
"SharedEndpointResolver" to types.sharedEndpointResolver,
"SmithyResolver" to types.resolveEndpoint,
"Params" to typesGenerator.paramsStruct(),
)
when (section) {
is ServiceConfig.ConfigStruct -> rustTemplate(
"pub (crate) endpoint_resolver: std::sync::Arc<dyn $resolverTrait>,",
"pub (crate) endpoint_resolver: $sharedEndpointResolver,",
*codegenScope,
)

is ServiceConfig.ConfigImpl ->
rustTemplate(
"""
/// Returns the endpoint resolver.
pub fn endpoint_resolver(&self) -> std::sync::Arc<dyn $resolverTrait> {
pub fn endpoint_resolver(&self) -> $sharedEndpointResolver {
self.endpoint_resolver.clone()
}
""",
Expand All @@ -52,7 +56,7 @@ internal class EndpointConfigCustomization(

is ServiceConfig.BuilderStruct ->
rustTemplate(
"endpoint_resolver: Option<std::sync::Arc<dyn $resolverTrait>>,",
"endpoint_resolver: #{Option}<$sharedEndpointResolver>,",
*codegenScope,
)

Expand Down Expand Up @@ -99,15 +103,15 @@ internal class EndpointConfigCustomization(
/// Sets the endpoint resolver to use when making requests.
$defaultResolverDocs
pub fn endpoint_resolver(mut self, endpoint_resolver: impl $resolverTrait + 'static) -> Self {
self.endpoint_resolver = Some(std::sync::Arc::new(endpoint_resolver) as _);
self.endpoint_resolver = #{Some}(#{SharedEndpointResolver}::new(endpoint_resolver));
self
}
/// Sets the endpoint resolver to use when making requests.
///
/// When unset, the client will used a generated endpoint resolver based on the endpoint resolution
/// rules for `$moduleUseName`.
pub fn set_endpoint_resolver(&mut self, endpoint_resolver: Option<std::sync::Arc<dyn $resolverTrait>>) -> &mut Self {
pub fn set_endpoint_resolver(&mut self, endpoint_resolver: #{Option}<$sharedEndpointResolver>) -> &mut Self {
self.endpoint_resolver = endpoint_resolver;
self
}
Expand All @@ -122,7 +126,7 @@ internal class EndpointConfigCustomization(
rustTemplate(
"""
endpoint_resolver: self.endpoint_resolver.unwrap_or_else(||
std::sync::Arc::new(#{DefaultResolver}::new())
#{SharedEndpointResolver}::new(#{DefaultResolver}::new())
),
""",
*codegenScope,
Expand Down Expand Up @@ -150,8 +154,9 @@ internal class EndpointConfigCustomization(
// always fail. In the future, this will be changed to an `expect()`
rustTemplate(
"""
endpoint_resolver: self.endpoint_resolver.unwrap_or_else(||std::sync::Arc::new(#{FailingResolver})),
endpoint_resolver: self.endpoint_resolver.unwrap_or_else(||#{SharedEndpointResolver}::new(#{FailingResolver})),
""",
*codegenScope,
"FailingResolver" to alwaysFailsResolver,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,14 @@ class EndpointsDecorator : ClientCodegenDecorator {
val codegenScope = arrayOf(
*RuntimeType.preludeScope,
"Params" to typesGenerator.paramsStruct(),
"ResolveEndpoint" to types.resolveEndpoint,
"ResolveEndpointError" to types.resolveEndpointError,
)
return when (section) {
is OperationSection.MutateInput -> writable {
rustTemplate(
"""
use #{ResolveEndpoint};
let params_result = #{Params}::builder()#{builderFields:W}.build()
.map_err(|err| #{ResolveEndpointError}::from_source("could not construct endpoint parameters", err));
let (endpoint_result, params) = match params_result {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Types(runtimeConfig: RuntimeConfig) {
private val smithyTypesEndpointModule = RuntimeType.smithyTypes(runtimeConfig).resolve("endpoint")
val smithyHttpEndpointModule = RuntimeType.smithyHttp(runtimeConfig).resolve("endpoint")
val resolveEndpoint = smithyHttpEndpointModule.resolve("ResolveEndpoint")
val sharedEndpointResolver = smithyHttpEndpointModule.resolve("SharedEndpointResolver")
val smithyEndpoint = smithyTypesEndpointModule.resolve("Endpoint")
val resolveEndpointError = smithyHttpEndpointModule.resolve("ResolveEndpointError")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class ServiceRuntimePluginGenerator(
"Params" to endpointTypesGenerator.paramsStruct(),
"ResolveEndpoint" to http.resolve("endpoint::ResolveEndpoint"),
"RuntimePlugin" to runtimeApi.resolve("client::runtime_plugin::RuntimePlugin"),
"SharedEndpointResolver" to http.resolve("endpoint::SharedEndpointResolver"),
"StaticAuthOptionResolver" to runtimeApi.resolve("client::auth::option_resolver::StaticAuthOptionResolver"),
"default_connector" to client.resolve("conns::default_connector"),
"require_connector" to client.resolve("conns::require_connector"),
Expand Down Expand Up @@ -134,7 +133,7 @@ class ServiceRuntimePluginGenerator(
cfg.set_auth_option_resolver(#{StaticAuthOptionResolver}::new(#{Vec}::new()));
let endpoint_resolver = #{DefaultEndpointResolver}::<#{Params}>::new(
#{SharedEndpointResolver}::from(self.handle.conf.endpoint_resolver()));
self.handle.conf.endpoint_resolver());
cfg.set_endpoint_resolver(endpoint_resolver);
// TODO(enableNewSmithyRuntime): Use the `store_append` method of ConfigBag to insert classifiers
Expand Down

0 comments on commit ec45767

Please sign in to comment.