Skip to content

Commit dae6b26

Browse files
authored
Update Jmespath shape traversal codegen to support multi-select lists following projection expressions (#3987)
## Motivation and Context Adds support for JMESPath multi-select lists following projection expressions such as `lists.structs[].[optionalInt, requiredInt]` (list projection followed by multi-select lists), `lists.structs[<some filter condition>].[optionalInt, requiredInt]` (filter projection followed by multi-select lists), and `maps.*.[optionalInt, requiredInt]` (object projection followed by multi-select lists). ## Description This PR adds support for the said functionality. Prior to the PR, the expressions above ended up the codegen either failing to generate code (for list projection) or generating the incorrect Rust code (for filter & object projection). All the code changes except for `RustJmespathShapeTraversalGenerator.kt` are primarily adjusting the existing code based on the updates made to `RustJmespathShapeTraversalGenerator.kt`. The gist of the code changes in `RustJmespathShapeTraversalGenerator.kt` is as follows: - `generateProjection` now supports `MultiSelectListExpression` on the right-hand side (RHS). - Previously, given `MultiSelectListExpression` on RHS, the `map` function applied to the result of the left-hand side of a projection expression (regardless of whether it's list, filter, or object projection) returned a type `Option<Vec<&T>>`, and the `map` function body used the `?` operator to return early as soon as it encountered a field value that was `None`. That did not yield the desired behavior. Given the snippet `lists.structs[].[optionalInt, requiredInt]` in the `Motivation and Context` above for instance, the `map` function used to look like this: ``` fn map(_v: &crate::types::Struct) -> Option<Vec<&i32>> { let _fld_1 = _v.optional_int.as_ref()?; let _fld_2 = _v.required_int; let _msl = vec![_fld_1, _fld_2]; Some(_msl) ``` This meant if the `optional_int` in a `Struct` was `None`, we lost the chance to access the `required_int` field when we should've. Instead, the `map` function now looks like: ``` fn map(_v: &crate::types::Struct) -> Option<Vec<Option<&i32>>> { let _fld_1 = _v.optional_int.as_ref(); let _fld_2 = Some(_v.required_int); let _msl = vec![_fld_1, _fld_2]; Some(_msl) ``` This way, the `map` function body has a chance to access all the fields of `Struct` even when any of the optional fields in a `Struct` is `None`. - Given the update to the signature of the `map` function above, `generate*Projection` functions have adjusted their implementations (such as [preserving the output type of the whole projection expression](https://github.com/smithy-lang/smithy-rs/blob/01fed784d5fc2ef6743a336496d91a51f01e6ab2/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/waiters/RustJmespathShapeTraversalGenerator.kt#L989-L1010) and performing [additional flattening for `Option`s](https://github.com/smithy-lang/smithy-rs/blob/01fed784d5fc2ef6743a336496d91a51f01e6ab2/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/waiters/RustJmespathShapeTraversalGenerator.kt#L757-L760)). Note that the output type of the whole projection expression stays the same before and after this PR; it's just that the inner `map` function used by the projection expression has been tweaked. ## Testing - Confirmed existing tests continued working (CI and release pipeline). - Added additional JMESPath codegen tests in `RustJmespathShapeTraversalGeneratorTest.kt`. - Confirmed that the updated service model with a JMESPath expression like `Items[*].[A.Name, B.Name, C.Name, D.Name][]` generated the expected Rust code and behaved as expected. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent b36447e commit dae6b26

File tree

9 files changed

+350
-121
lines changed

9 files changed

+350
-121
lines changed

aws/sdk/integration-tests/dynamodb/tests/test-error-classification.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ async fn assert_error_not_transient(error: ReplayedEvent) {
6262
let client = Client::from_conf(config);
6363
let _item = client
6464
.get_item()
65+
.table_name("arn:aws:dynamodb:us-east-2:333333333333:table/table_name")
6566
.key("foo", AttributeValue::Bool(true))
6667
.send()
6768
.await

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.config.confi
2323
import software.amazon.smithy.rust.codegen.client.smithy.generators.config.loadFromConfigBag
2424
import software.amazon.smithy.rust.codegen.client.smithy.generators.waiters.RustJmespathShapeTraversalGenerator
2525
import software.amazon.smithy.rust.codegen.client.smithy.generators.waiters.TraversalBinding
26+
import software.amazon.smithy.rust.codegen.client.smithy.generators.waiters.TraversalContext
2627
import software.amazon.smithy.rust.codegen.client.smithy.generators.waiters.TraversedShape
2728
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
2829
import software.amazon.smithy.rust.codegen.core.rustlang.RustType
@@ -168,16 +169,18 @@ class EndpointParamsInterceptorGenerator(
168169
TraversedShape.from(model, operationShape.inputShape(model)),
169170
),
170171
),
172+
TraversalContext(retainOption = false),
171173
)
172174

173175
when (pathTraversal.outputType) {
174176
is RustType.Vec -> {
175-
rust(".$setterName($getterName(_input))")
176-
}
177-
178-
else -> {
179-
rust(".$setterName($getterName(_input).cloned())")
177+
if (pathTraversal.outputType.member is RustType.Reference) {
178+
rust(".$setterName($getterName(_input).map(|v| v.into_iter().cloned().collect::<Vec<_>>()))")
179+
} else {
180+
rust(".$setterName($getterName(_input))")
181+
}
180182
}
183+
else -> rust(".$setterName($getterName(_input).cloned())")
181184
}
182185
}
183186

@@ -211,6 +214,7 @@ class EndpointParamsInterceptorGenerator(
211214
TraversedShape.from(model, operationShape.inputShape(model)),
212215
),
213216
),
217+
TraversalContext(retainOption = false),
214218
)
215219

216220
rust("// Generated from JMESPath Expression: $pathValue")

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ internal class EndpointResolverGenerator(
154154
"clippy::comparison_to_empty",
155155
// we generate `if let Some(_) = ... { ... }`
156156
"clippy::redundant_pattern_matching",
157+
// we generate `if (s.as_ref() as &str) == ("arn:") { ... }`, and `s` can be either `String` or `&str`
158+
"clippy::useless_asref",
157159
)
158160
private val context = Context(registry, runtimeConfig)
159161

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import software.amazon.smithy.rulesengine.traits.ExpectedEndpoint
1818
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
1919
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.Types
2020
import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName
21-
import software.amazon.smithy.rust.codegen.client.smithy.generators.ClientInstantiator
2221
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
2322
import software.amazon.smithy.rust.codegen.core.rustlang.docs
2423
import software.amazon.smithy.rust.codegen.core.rustlang.escape
@@ -50,8 +49,6 @@ internal class EndpointTestGenerator(
5049
"capture_request" to RuntimeType.captureRequest(runtimeConfig),
5150
)
5251

53-
private val instantiator = ClientInstantiator(codegenContext)
54-
5552
private fun EndpointTestCase.docs(): Writable {
5653
val self = this
5754
return writable { docs(self.documentation.orElse("no docs")) }
@@ -134,7 +131,23 @@ internal class EndpointTestGenerator(
134131
value.values.map { member ->
135132
writable {
136133
rustTemplate(
137-
"#{Document}::from(#{value:W})",
134+
/*
135+
* If we wrote "#{Document}::from(#{value:W})" here, we could encounter a
136+
* compile error due to the following type mismatch:
137+
* the trait `From<Vec<Document>>` is not implemented for `Vec<std::string::String>`
138+
*
139+
* given the following method signature:
140+
* fn resource_arn_list(mut self, value: impl Into<::std::vec::Vec<::std::string::String>>)
141+
*
142+
* with a call site like this:
143+
* .resource_arn_list(vec![::aws_smithy_types::Document::from(
144+
* "arn:aws:dynamodb:us-east-1:333333333333:table/table_name".to_string(),
145+
* )])
146+
*
147+
* For this reason we use `into()` instead to allow types that need to be converted
148+
* to `Document` to continue working as before, and to support the above use case.
149+
*/
150+
"#{value:W}.into()",
138151
*codegenScope,
139152
"value" to generateValue(member),
140153
)

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/ExpressionGenerator.kt

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.endpoint.rulesgen
88
import org.jetbrains.annotations.Contract
99
import software.amazon.smithy.rulesengine.language.evaluation.type.BooleanType
1010
import software.amazon.smithy.rulesengine.language.evaluation.type.OptionalType
11+
import software.amazon.smithy.rulesengine.language.evaluation.type.StringType
1112
import software.amazon.smithy.rulesengine.language.evaluation.type.Type
1213
import software.amazon.smithy.rulesengine.language.syntax.expressions.Expression
1314
import software.amazon.smithy.rulesengine.language.syntax.expressions.ExpressionVisitor
@@ -24,6 +25,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rust
2425
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
2526
import software.amazon.smithy.rust.codegen.core.rustlang.writable
2627
import software.amazon.smithy.rust.codegen.core.util.PANIC
28+
import java.lang.RuntimeException
2729

2830
/**
2931
* Root expression generator.
@@ -56,7 +58,18 @@ class ExpressionGenerator(
5658
else -> rust("${ref.name.rustName()}.to_owned()")
5759
}
5860
} else {
59-
rust(ref.name.rustName())
61+
try {
62+
when (ref.type()) {
63+
// This ensures we obtain a `&str`, regardless of whether `ref.name.rustName()` returns a `String` or a `&str`.
64+
// Typically, we don't know which type will be returned due to code generation.
65+
is StringType -> rust("${ref.name.rustName()}.as_ref() as &str")
66+
else -> rust(ref.name.rustName())
67+
}
68+
} catch (_: RuntimeException) {
69+
// Because Typechecking was never invoked upon calling `.type()` on Reference for an expression
70+
// like "{ref}: rust". See `generateLiterals2` in ExprGeneratorTest.
71+
rust(ref.name.rustName())
72+
}
6073
}
6174
}
6275

0 commit comments

Comments
 (0)