Skip to content

Commit bb8e19a

Browse files
ysaito1001ysaito1001
authored
Remove HACK that used to put Handle in config bag (#2806)
## Motivation and Context Removes `HACK` that used to put `Handle` in config bag. ## Description The `HACK` was previously placed in `ServiceRuntimePlugin::config` because later in the orchestrator execution, we needed to access service config fields (through `Handle`) to build endpoint `Params` within `EndpointParamsInterceptor`. Now that service config fields are baked into a frozen layer, `EndpointParamsInterceptor` can load those fields directly from the config bag (to which the frozen layer has been added) when constructing endpoint `Params`. We can therefore remove `HACK` at this point. ## Testing - [x] Passed tests in CI ---- _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 <awsaito@amazon.com>
1 parent 177965c commit bb8e19a

File tree

7 files changed

+118
-63
lines changed

7 files changed

+118
-63
lines changed

‎aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/EndpointBuiltInsDecorator.kt

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,23 @@ import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegen
2020
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointCustomization
2121
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointRulesetIndex
2222
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName
23+
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.symbol
2324
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization
2425
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigParam
2526
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.configParamNewtype
27+
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.loadFromConfigBag
2628
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.standardConfigParam
29+
import software.amazon.smithy.rust.codegen.core.rustlang.RustType
2730
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
2831
import software.amazon.smithy.rust.codegen.core.rustlang.docs
2932
import software.amazon.smithy.rust.codegen.core.rustlang.rust
3033
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
34+
import software.amazon.smithy.rust.codegen.core.rustlang.stripOuter
3135
import software.amazon.smithy.rust.codegen.core.rustlang.writable
3236
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
3337
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
3438
import software.amazon.smithy.rust.codegen.core.smithy.customize.AdHocCustomization
39+
import software.amazon.smithy.rust.codegen.core.smithy.mapRustType
3540
import software.amazon.smithy.rust.codegen.core.util.PANIC
3641
import software.amazon.smithy.rust.codegen.core.util.dq
3742
import software.amazon.smithy.rust.codegen.core.util.extendIf
@@ -54,22 +59,20 @@ fun ClientCodegenContext.getBuiltIn(builtIn: String): Parameter? {
5459
private fun promotedBuiltins(parameter: Parameter) =
5560
parameter == Builtins.FIPS || parameter == Builtins.DUALSTACK || parameter == Builtins.SDK_ENDPOINT
5661

62+
private fun configParamNewtype(parameter: Parameter, name: String, runtimeConfig: RuntimeConfig): RuntimeType {
63+
val type = parameter.symbol().mapRustType { t -> t.stripOuter<RustType.Option>() }
64+
return when (promotedBuiltins(parameter)) {
65+
true -> AwsRuntimeType.awsTypes(runtimeConfig)
66+
.resolve("endpoint_config::${name.toPascalCase()}")
67+
68+
false -> configParamNewtype(name.toPascalCase(), type, runtimeConfig)
69+
}
70+
}
71+
5772
private fun ConfigParam.Builder.toConfigParam(parameter: Parameter, runtimeConfig: RuntimeConfig): ConfigParam =
5873
this.name(this.name ?: parameter.name.rustName())
59-
.type(
60-
when (parameter.type!!) {
61-
ParameterType.STRING -> RuntimeType.String.toSymbol()
62-
ParameterType.BOOLEAN -> RuntimeType.Bool.toSymbol()
63-
},
64-
)
65-
.newtype(
66-
when (promotedBuiltins(parameter)) {
67-
true -> AwsRuntimeType.awsTypes(runtimeConfig)
68-
.resolve("endpoint_config::${this.name!!.toPascalCase()}")
69-
70-
false -> configParamNewtype(this.name!!.toPascalCase(), this.type!!, runtimeConfig)
71-
},
72-
)
74+
.type(parameter.symbol().mapRustType { t -> t.stripOuter<RustType.Option>() })
75+
.newtype(configParamNewtype(parameter, this.name!!, runtimeConfig))
7376
.setterDocs(this.setterDocs ?: parameter.documentation.orNull()?.let { writable { docs(it) } })
7477
.build()
7578

@@ -139,7 +142,12 @@ fun decoratorForBuiltIn(
139142
when (parameter.builtIn) {
140143
builtIn.builtIn -> writable {
141144
if (codegenContext.smithyRuntimeMode.defaultToOrchestrator) {
142-
rust("$configRef.$name()")
145+
val newtype = configParamNewtype(parameter, name, codegenContext.runtimeConfig)
146+
val symbol = parameter.symbol().mapRustType { t -> t.stripOuter<RustType.Option>() }
147+
rustTemplate(
148+
"""$configRef.#{load_from_service_config_layer}""",
149+
"load_from_service_config_layer" to loadFromConfigBag(symbol.name, newtype),
150+
)
143151
} else {
144152
rust("$configRef.$name")
145153
}

‎aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RegionDecorator.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,10 @@ class RegionDecorator : ClientCodegenDecorator {
132132
return when (parameter.builtIn) {
133133
Builtins.REGION.builtIn -> writable {
134134
if (codegenContext.smithyRuntimeMode.defaultToOrchestrator) {
135-
rust("$configRef.region().as_ref().map(|r|r.as_ref().to_owned())")
135+
rustTemplate(
136+
"$configRef.load::<#{Region}>().map(|r|r.as_ref().to_owned())",
137+
"Region" to region(codegenContext.runtimeConfig).resolve("Region"),
138+
)
136139
} else {
137140
rust("$configRef.region.as_ref().map(|r|r.as_ref().to_owned())")
138141
}

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ import software.amazon.smithy.model.traits.EndpointTrait
1313
import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameters
1414
import software.amazon.smithy.rulesengine.traits.ContextIndex
1515
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
16+
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.ClientContextConfigCustomization
1617
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointTypesGenerator
1718
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName
1819
import software.amazon.smithy.rust.codegen.client.smithy.generators.EndpointTraitBindings
20+
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.configParamNewtype
21+
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.loadFromConfigBag
1922
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
2023
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
2124
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
@@ -83,11 +86,6 @@ class EndpointParamsInterceptorGenerator(
8386
8487
#{endpoint_prefix:W}
8588
86-
// HACK: pull the handle out of the config bag until config is implemented right
87-
let handle = cfg.get::<std::sync::Arc<crate::client::Handle>>()
88-
.expect("the handle is hacked into the config bag");
89-
let _config = &handle.conf;
90-
9189
let params = #{Params}::builder()
9290
#{param_setters}
9391
.build()
@@ -109,15 +107,24 @@ class EndpointParamsInterceptorGenerator(
109107
val builtInParams = params.toList().filter { it.isBuiltIn }
110108
// first load builtins and their defaults
111109
builtInParams.forEach { param ->
112-
endpointTypesGenerator.builtInFor(param, "_config")?.also { defaultValue ->
110+
val config = if (codegenContext.smithyRuntimeMode.defaultToOrchestrator) {
111+
"cfg"
112+
} else {
113+
"_config"
114+
}
115+
endpointTypesGenerator.builtInFor(param, config)?.also { defaultValue ->
113116
rust(".set_${param.name.rustName()}(#W)", defaultValue)
114117
}
115118
}
116119

117120
idx.getClientContextParams(codegenContext.serviceShape).orNull()?.parameters?.forEach { (name, param) ->
118-
val paramName = EndpointParamsGenerator.memberName(name)
119121
val setterName = EndpointParamsGenerator.setterName(name)
120-
rust(".$setterName(_config.$paramName())")
122+
val inner = ClientContextConfigCustomization.toSymbol(param.type, symbolProvider)
123+
val newtype = configParamNewtype(name, inner, codegenContext.runtimeConfig)
124+
rustTemplate(
125+
".$setterName(cfg.#{load_from_service_config_layer})",
126+
"load_from_service_config_layer" to loadFromConfigBag(inner.name, newtype),
127+
)
121128
}
122129

123130
idx.getStaticContextParams(operationShape).orNull()?.parameters?.forEach { (name, param) ->

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ class ServiceRuntimePluginGenerator(
111111
fun render(writer: RustWriter, customizations: List<ServiceRuntimePluginCustomization>) {
112112
writer.rustTemplate(
113113
"""
114+
// TODO(enableNewSmithyRuntimeLaunch) Remove `allow(dead_code)` as well as a field `handle` when
115+
// the field is no longer used.
116+
##[allow(dead_code)]
114117
##[derive(Debug)]
115118
pub(crate) struct ServiceRuntimePlugin {
116119
handle: #{Arc}<crate::client::Handle>,
@@ -127,9 +130,6 @@ class ServiceRuntimePluginGenerator(
127130
use #{ConfigBagAccessors};
128131
let mut cfg = #{Layer}::new(${codegenContext.serviceShape.id.name.dq()});
129132
130-
// HACK: Put the handle into the config bag to work around config not being fully implemented yet
131-
cfg.put(self.handle.clone());
132-
133133
let http_auth_schemes = #{HttpAuthSchemes}::builder()
134134
#{http_auth_scheme_customizations}
135135
.build();

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import software.amazon.smithy.model.shapes.ServiceShape
1313
import software.amazon.smithy.model.traits.IdempotencyTokenTrait
1414
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
1515
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule
16+
import software.amazon.smithy.rust.codegen.client.smithy.customizations.codegenScope
1617
import software.amazon.smithy.rust.codegen.client.smithy.customize.TestUtilFeature
1718
import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
1819
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
@@ -159,15 +160,37 @@ fun configParamNewtype(newtypeName: String, inner: Symbol, runtimeConfig: Runtim
159160
rustTemplate(
160161
"""
161162
##[derive(Debug, Clone)]
162-
pub(crate) struct $newtypeName($inner);
163+
pub(crate) struct $newtypeName(pub(crate) $inner);
163164
impl #{Storable} for $newtypeName {
164-
type Storer = #{StoreReplace}<$newtypeName>;
165+
type Storer = #{StoreReplace}<Self>;
165166
}
166167
""",
167168
*codegenScope,
168169
)
169170
}
170171

172+
/**
173+
* Render an expression that loads a value from a config bag.
174+
*
175+
* The expression to be rendered handles a case where a newtype is stored in the config bag, but the user expects
176+
* the underlying raw type after the newtype has been loaded from the bag.
177+
*/
178+
fun loadFromConfigBag(innerTypeName: String, newtype: RuntimeType): Writable = writable {
179+
rustTemplate(
180+
"""
181+
load::<#{newtype}>().map(#{f})
182+
""",
183+
"newtype" to newtype,
184+
"f" to writable {
185+
if (innerTypeName == "bool") {
186+
rust("|ty| ty.0")
187+
} else {
188+
rust("|ty| ty.0.clone()")
189+
}
190+
},
191+
)
192+
}
193+
171194
/**
172195
* Config customization for a config param with no special behavior:
173196
* 1. `pub(crate)` field
@@ -190,31 +213,6 @@ fun standardConfigParam(param: ConfigParam, codegenContext: ClientCodegenContext
190213
}
191214
}
192215

193-
ServiceConfig.ConfigImpl -> writable {
194-
if (runtimeMode.defaultToOrchestrator) {
195-
rustTemplate(
196-
"""
197-
pub(crate) fn ${param.name}(&self) -> #{output} {
198-
self.inner.load::<#{newtype}>().map(#{f})
199-
}
200-
""",
201-
"f" to writable {
202-
if (param.type.name == "bool") {
203-
rust("|ty| ty.0")
204-
} else {
205-
rust("|ty| ty.0.clone()")
206-
}
207-
},
208-
"newtype" to param.newtype!!,
209-
"output" to if (param.optional) {
210-
param.type.makeOptional()
211-
} else {
212-
param.type
213-
},
214-
)
215-
}
216-
}
217-
218216
ServiceConfig.BuilderStruct -> writable {
219217
if (runtimeMode.defaultToMiddleware) {
220218
rust("${param.name}: #T,", param.type.makeOptional())
@@ -430,6 +428,7 @@ class ServiceConfigGenerator(
430428
rustBlock("pub fn build(mut self) -> Config") {
431429
rustTemplate(
432430
"""
431+
##[allow(unused_imports)]
433432
use #{ConfigBagAccessors};
434433
// The builder is being turned into a service config. While doing so, we'd like to avoid
435434
// requiring that items created and stored _during_ the build method be `Clone`, since they
@@ -472,14 +471,17 @@ class ServiceConfigGenerator(
472471
#{Some}(self.inner.clone())
473472
}
474473
475-
fn interceptors(&self, interceptors: &mut #{InterceptorRegistrar}) {
476-
interceptors.extend(self.interceptors.iter().cloned());
474+
fn interceptors(&self, _interceptors: &mut #{InterceptorRegistrar}) {
475+
#{interceptors}
477476
}
478477
}
479478
480479
""",
481480
*codegenScope,
482481
"config" to writable { writeCustomizations(customizations, ServiceConfig.RuntimePluginConfig("cfg")) },
482+
"interceptors" to writable {
483+
writeCustomizations(customizations, ServiceConfig.RuntimePluginInterceptors("_interceptors"))
484+
},
483485
)
484486
}
485487

‎codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/testutil/TestConfigCustomization.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ fun stubConfigProject(codegenContext: ClientCodegenContext, customization: Confi
129129
val generator = ServiceConfigGenerator(codegenContext, customizations = customizations.toList())
130130
project.withModule(ClientRustModule.config) {
131131
generator.render(this)
132+
if (codegenContext.smithyRuntimeMode.defaultToOrchestrator) {
133+
generator.renderRuntimePluginImplForSelf(this)
134+
}
132135
unitTest(
133136
"config_send_sync",
134137
"""

‎codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/ClientContextConfigCustomizationTest.kt

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import software.amazon.smithy.rust.codegen.client.testutil.testClientCodegenCont
1212
import software.amazon.smithy.rust.codegen.client.testutil.validateConfigCustomizations
1313
import software.amazon.smithy.rust.codegen.client.testutil.withSmithyRuntimeMode
1414
import software.amazon.smithy.rust.codegen.core.rustlang.rust
15+
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
16+
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
1517
import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace
1618
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
1719
import software.amazon.smithy.rust.codegen.core.testutil.unitTest
@@ -37,14 +39,30 @@ class ClientContextConfigCustomizationTest {
3739
fun `client params generate a valid customization`(smithyRuntimeModeStr: String) {
3840
val project = TestWorkspace.testProject()
3941
val smithyRuntimeMode = SmithyRuntimeMode.fromString(smithyRuntimeModeStr)
42+
val context = testClientCodegenContext(model).withSmithyRuntimeMode(smithyRuntimeMode)
4043
project.unitTest {
4144
if (smithyRuntimeMode.defaultToOrchestrator) {
42-
rust(
45+
rustTemplate(
4346
"""
47+
use #{RuntimePlugin};
4448
let conf = crate::Config::builder().a_string_param("hello!").a_bool_param(true).build();
45-
assert_eq!(conf.a_string_param().unwrap(), "hello!");
46-
assert_eq!(conf.a_bool_param(), Some(true));
49+
assert_eq!(
50+
conf.config()
51+
.unwrap()
52+
.load::<crate::config::AStringParam>()
53+
.map(|u| u.0.clone())
54+
.unwrap(),
55+
"hello!"
56+
);
57+
assert_eq!(
58+
conf.config()
59+
.unwrap()
60+
.load::<crate::config::ABoolParam>()
61+
.map(|u| u.0),
62+
Some(true)
63+
);
4764
""",
65+
"RuntimePlugin" to RuntimeType.runtimePlugin(context.runtimeConfig),
4866
)
4967
} else {
5068
rust(
@@ -59,12 +77,27 @@ class ClientContextConfigCustomizationTest {
5977
// unset fields
6078
project.unitTest {
6179
if (smithyRuntimeMode.defaultToOrchestrator) {
62-
rust(
80+
rustTemplate(
6381
"""
82+
use #{RuntimePlugin};
6483
let conf = crate::Config::builder().a_string_param("hello!").build();
65-
assert_eq!(conf.a_string_param().unwrap(), "hello!");
66-
assert_eq!(conf.a_bool_param(), None);
84+
assert_eq!(
85+
conf.config()
86+
.unwrap()
87+
.load::<crate::config::AStringParam>()
88+
.map(|u| u.0.clone())
89+
.unwrap(),
90+
"hello!"
91+
);
92+
assert_eq!(
93+
conf.config()
94+
.unwrap()
95+
.load::<crate::config::ABoolParam>()
96+
.map(|u| u.0),
97+
None,
98+
);
6799
""",
100+
"RuntimePlugin" to RuntimeType.runtimePlugin(context.runtimeConfig),
68101
)
69102
} else {
70103
rust(
@@ -76,7 +109,6 @@ class ClientContextConfigCustomizationTest {
76109
)
77110
}
78111
}
79-
val context = testClientCodegenContext(model).withSmithyRuntimeMode(smithyRuntimeMode)
80112
validateConfigCustomizations(context, ClientContextConfigCustomization(context), project)
81113
}
82114
}

0 commit comments

Comments
 (0)