Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for nullable struct members when generating AWS SDKs #2916

Merged
merged 43 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9c17050
add support for nullable struct members when generating AWS SDKs
Velfi Aug 10, 2023
b1c7ec5
add CHANGELOG.next.toml entries
Velfi Aug 10, 2023
0f27839
fix SDK adhoc tests config generation
Velfi Aug 10, 2023
81f428b
fix XML parseStructure
Velfi Aug 10, 2023
633c257
Merge branch 'main' into zhessler-nullability-now!
Velfi Aug 14, 2023
266f54f
fix server codegen issue
Velfi Aug 14, 2023
86dc322
remove TODO
Velfi Aug 14, 2023
a5565a3
update route53 resource ID trimmer
Velfi Aug 14, 2023
2d20462
add back the prefix loop that I accidentally removed
Velfi Aug 14, 2023
acff7bd
more fixing for route53
Velfi Aug 14, 2023
976f3a2
fix query serializer generator ref
Velfi Aug 14, 2023
397c4ab
fix the QuerySerializerGenerator correctly
Velfi Aug 14, 2023
80bb768
fix idempotency token generation
Velfi Aug 15, 2023
a44ae99
no really, fix idempotency token generation
Velfi Aug 15, 2023
916265a
fix serde bugs
Velfi Aug 16, 2023
761c1e4
fix more tests
Velfi Aug 16, 2023
36b7127
fix server client and eventstream stuff
Velfi Aug 16, 2023
3cf8288
Merge branch 'main' into zhessler-nullability-now!
Velfi Aug 16, 2023
7e464cc
remove insurmountable comma
Velfi Aug 16, 2023
d2b2c55
fix another server example
Velfi Aug 16, 2023
505c778
Merge branch 'main' into zhessler-nullability-now!
Velfi Aug 17, 2023
f0e4a1f
fix gradle build config typo
Velfi Aug 17, 2023
ae6a636
Update codegen-client/src/main/kotlin/software/amazon/smithy/rust/cod…
Velfi Aug 17, 2023
491ca43
respond to PR comments
Velfi Aug 17, 2023
66fbddb
Merge branch 'main' into zhessler-nullability-now!
Velfi Aug 17, 2023
c4b9958
add missing author to changelog
Velfi Aug 17, 2023
dac4679
update CHANGELOG
Velfi Aug 17, 2023
b0c63c9
Merge branch 'main' of github.com:awslabs/smithy-rs into zhessler-nul…
rcoh Sep 11, 2023
5e6f6b9
Fix endpoints decorator test
rcoh Sep 12, 2023
94c8dbb
Create ClientBuilderInstatiator & manuall create BuildError
rcoh Sep 12, 2023
cd5e33c
Update ChangeLog
rcoh Sep 12, 2023
1374e95
Fix server
rcoh Sep 12, 2023
e661075
Merge branch 'main' of github.com:awslabs/smithy-rs into zhessler-nul…
rcoh Sep 20, 2023
09c02f7
Fix unit tests
rcoh Sep 20, 2023
b417977
Add a default instantiator that works for tests
rcoh Sep 20, 2023
0bffcc0
add protocol test for error correction & nullability
rcoh Sep 20, 2023
eee07db
Merge branch 'main' of github.com:awslabs/smithy-rs into zhessler-nul…
rcoh Sep 20, 2023
58cf184
Add nullability test and fix some bugs
rcoh Sep 20, 2023
7a4fd41
A few more fixes
rcoh Sep 21, 2023
e75ad16
wip
rcoh Sep 21, 2023
99de898
Fix request id accessor when it is marked as required
rcoh Sep 21, 2023
969e32f
Refactor && remove println
rcoh Sep 21, 2023
7de1892
Fix unit test & add an ignore for useless question mark
rcoh Sep 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ references = ["smithy-rs#2911"]
meta = { "breaking" = true, "tada" = false, "bug" = false }
author = "Velfi"

[[aws-sdk-rust]]
message = "Struct members modeled as required are no longer wrapped in `Option`s [when possible](https://smithy.io/2.0/spec/aggregate-types.html#structure-member-optionality). For upgrade guidance and more info, see [here](https://github.com/awslabs/smithy-rs/discussions/2929)."
references = ["smithy-rs#2916", "aws-sdk-rust#536"]
meta = { "breaking" = true, "tada" = true, "bug" = false }
author = "Velfi"

[[smithy-rs]]
message = """
Support for Smithy IDLv2 nullability is now enabled by default. You can maintain the old behavior by setting `nullabilityCheckMode: "CLIENT_ZERO_VALUE_V1" in your codegen config.
For upgrade guidance and more info, see [here](https://github.com/awslabs/smithy-rs/discussions/2929).
"""
references = ["smithy-rs#2916", "smithy-rs#1767"]
meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "client"}
author = "Velfi"

[[aws-sdk-rust]]
message = """
All versions of SigningParams have been updated to contain an [`Identity`](https://docs.rs/aws-smithy-runtime-api/latest/aws_smithy_runtime_api/client/identity/struct.Identity.html)
Expand Down
17 changes: 4 additions & 13 deletions aws/rust-runtime/aws-config/src/sts/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,15 @@ pub(crate) fn into_credentials(
) -> provider::Result {
let sts_credentials = sts_credentials
.ok_or_else(|| CredentialsError::unhandled("STS credentials must be defined"))?;
let expiration = SystemTime::try_from(
sts_credentials
.expiration
.ok_or_else(|| CredentialsError::unhandled("missing expiration"))?,
)
.map_err(|_| {
let expiration = SystemTime::try_from(sts_credentials.expiration).map_err(|_| {
CredentialsError::unhandled(
"credential expiration time cannot be represented by a SystemTime",
)
})?;
Ok(AwsCredentials::new(
sts_credentials
.access_key_id
.ok_or_else(|| CredentialsError::unhandled("access key id missing from result"))?,
sts_credentials
.secret_access_key
.ok_or_else(|| CredentialsError::unhandled("secret access token missing"))?,
sts_credentials.session_token,
sts_credentials.access_key_id,
sts_credentials.secret_access_key,
Some(sts_credentials.session_token),
Some(expiration),
provider_name,
))
Expand Down
52 changes: 29 additions & 23 deletions aws/sdk-adhoc-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -35,40 +35,46 @@ dependencies {
implementation("software.amazon.smithy:smithy-aws-protocol-tests:$smithyVersion")
implementation("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-model:$smithyVersion")
}

val allCodegenTests = listOf(
CodegenTest(
"com.amazonaws.apigateway#BackplaneControlService",
"apigateway",
imports = listOf("models/apigateway-rules.smithy"),
fun getNullabilityCheckMode(): String = properties.get("nullability.check.mode") ?: "CLIENT_CAREFUL"

fun baseTest(service: String, module: String, imports: List<String> = listOf()): CodegenTest {
return CodegenTest(
service = service,
module = module,
imports = imports,
extraCodegenConfig = """
"includeFluentClient": false,
"nullabilityCheckMode": "${getNullabilityCheckMode()}"
""",
extraConfig = """
,
"codegen": {
"includeFluentClient": false
},
"customizationConfig": {
, "customizationConfig": {
"awsSdk": {
"generateReadme": false
"generateReadme": false,
"requireEndpointResolver": false
}
}
""",
)
}

val allCodegenTests = listOf(
baseTest(
"com.amazonaws.apigateway#BackplaneControlService",
"apigateway",
imports = listOf("models/apigateway-rules.smithy"),
),
CodegenTest(
baseTest(
"com.amazonaws.testservice#TestService",
"endpoint-test-service",
imports = listOf("models/single-static-endpoint.smithy"),
extraConfig = """
,
"codegen": {
"includeFluentClient": false
},
"customizationConfig": {
"awsSdk": {
"generateReadme": false
}
}
""",
),
baseTest(
"com.amazonaws.testservice#RequiredValues",
"required-values",
imports = listOf("models/required-value-test.smithy"),
),
)

Expand Down
28 changes: 28 additions & 0 deletions aws/sdk-adhoc-test/models/required-value-test.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
$version: "1.0"

namespace com.amazonaws.testservice

use aws.api#service
use aws.protocols#restJson1

@restJson1
@title("Test Service")
@service(sdkId: "Test")
@aws.auth#sigv4(name: "test-service")
service RequiredValues {
operations: [TestOperation]
}

@http(method: "GET", uri: "/")
operation TestOperation {
errors: [Error]
}

@error("client")
structure Error {
@required
requestId: String

@required
message: String
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

package software.amazon.smithy.rustsdk

import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
Expand All @@ -19,13 +21,15 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
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
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderCustomization
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderSection
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureCustomization
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureSection
import software.amazon.smithy.rust.codegen.core.smithy.generators.error.ErrorImplCustomization
import software.amazon.smithy.rust.codegen.core.smithy.generators.error.ErrorImplSection
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticOutputTrait
import software.amazon.smithy.rust.codegen.core.util.hasTrait

Expand Down Expand Up @@ -72,6 +76,10 @@ abstract class BaseRequestIdDecorator : ClientCodegenDecorator {
}
}

open fun asMemberShape(container: StructureShape): MemberShape? {
return container.members().firstOrNull { member -> member.memberName.lowercase() == "requestid" }
}

private inner class RequestIdOperationCustomization(private val codegenContext: ClientCodegenContext) :
OperationCustomization() {
override fun section(section: OperationSection): Writable = writable {
Expand All @@ -82,19 +90,22 @@ abstract class BaseRequestIdDecorator : ClientCodegenDecorator {
"apply_to_error" to applyToError(codegenContext),
)
}

is OperationSection.MutateOutput -> {
rust(
"output._set_$fieldName(#T::$accessorFunctionName(${section.responseHeadersName}).map(str::to_string));",
accessorTrait(codegenContext),
)
}

is OperationSection.BeforeParseResponse -> {
rustTemplate(
"#{tracing}::debug!($fieldName = ?#{trait}::$accessorFunctionName(${section.responseName}));",
"tracing" to RuntimeType.Tracing,
"trait" to accessorTrait(codegenContext),
)
}

else -> {}
}
}
Expand Down Expand Up @@ -123,8 +134,17 @@ abstract class BaseRequestIdDecorator : ClientCodegenDecorator {
rustBlock("fn $accessorFunctionName(&self) -> Option<&str>") {
rustBlock("match self") {
section.allErrors.forEach { error ->
val optional = asMemberShape(error)?.let { member ->
codegenContext.symbolProvider.toSymbol(member).isOptional()
} ?: true
val wrapped = writable {
when (optional) {
false -> rustTemplate("#{Some}(e.$accessorFunctionName())", *preludeScope)
true -> rustTemplate("e.$accessorFunctionName()")
}
}
val sym = codegenContext.symbolProvider.toSymbol(error)
rust("Self::${sym.name}(e) => e.$accessorFunctionName(),")
rust("Self::${sym.name}(e) => #T,", wrapped)
}
rust("Self::Unhandled(e) => e.$accessorFunctionName(),")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package software.amazon.smithy.rustsdk.customize.s3

import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rustsdk.BaseRequestIdDecorator
Expand All @@ -17,6 +19,10 @@ class S3ExtendedRequestIdDecorator : BaseRequestIdDecorator() {
override val fieldName: String = "extended_request_id"
override val accessorFunctionName: String = "extended_request_id"

override fun asMemberShape(container: StructureShape): MemberShape? {
return null
}

private val requestIdModule: RuntimeType =
RuntimeType.forInlineDependency(InlineAwsDependency.forRustFile("s3_request_id"))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ class TimestreamDecorator : ClientCodegenDecorator {
client.describe_endpoints().send().await.map_err(|e| {
#{ResolveEndpointError}::from_source("failed to call describe_endpoints", e)
})?;
let endpoint = describe_endpoints.endpoints().unwrap().get(0).unwrap();
let endpoint = describe_endpoints.endpoints().get(0).unwrap();
let expiry = client.config().time_source().expect("checked when ep discovery was enabled").now()
+ #{Duration}::from_secs(endpoint.cache_period_in_minutes() as u64 * 60);
Ok((
#{Endpoint}::builder()
.url(format!("https://{}", endpoint.address().unwrap()))
.url(format!("https://{}", endpoint.address()))
.build(),
expiry,
))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,22 @@
package software.amazon.smithy.rustsdk.customize.ec2

import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.CsvSource
import software.amazon.smithy.model.knowledge.NullableIndex
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.util.lookup

internal class EC2MakePrimitivesOptionalTest {
@Test
fun `primitive shapes are boxed`() {
@ParameterizedTest
@CsvSource(
"CLIENT",
"CLIENT_CAREFUL",
"CLIENT_ZERO_VALUE_V1",
"CLIENT_ZERO_VALUE_V1_NO_INPUT",
)
fun `primitive shapes are boxed`(nullabilityCheckMode: NullableIndex.CheckMode) {
val baseModel = """
namespace test
structure Primitives {
Expand All @@ -36,7 +43,7 @@ internal class EC2MakePrimitivesOptionalTest {
val nullableIndex = NullableIndex(model)
val struct = model.lookup<StructureShape>("test#Primitives")
struct.members().forEach {
nullableIndex.isMemberNullable(it, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1) shouldBe true
nullableIndex.isMemberNullable(it, nullabilityCheckMode) shouldBe true
}
}
}
4 changes: 3 additions & 1 deletion aws/sdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ val crateVersioner by lazy { aws.sdk.CrateVersioner.defaultFor(rootProject, prop

fun getRustMSRV(): String = properties.get("rust.msrv") ?: throw Exception("Rust MSRV missing")
fun getPreviousReleaseVersionManifestPath(): String? = properties.get("aws.sdk.previous.release.versions.manifest")
fun getNullabilityCheckMode(): String = properties.get("nullability.check.mode") ?: "CLIENT_CAREFUL"

fun loadServiceMembership(): Membership {
val membershipOverride = properties.get("aws.services")?.let { parseMembership(it) }
Expand Down Expand Up @@ -103,7 +104,8 @@ fun generateSmithyBuild(services: AwsServices): String {
"renameErrors": false,
"debugMode": $debugMode,
"eventStreamAllowList": [$eventStreamAllowListMembers],
"enableUserConfigurableRuntimePlugins": false
"enableUserConfigurableRuntimePlugins": false,
"nullabilityCheckMode": "${getNullabilityCheckMode()}"
},
"service": "${service.service}",
"module": "$moduleName",
Expand Down
2 changes: 1 addition & 1 deletion aws/sdk/integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ members = [
"s3",
"s3control",
"sts",
"transcribestreaming",
"timestreamquery",
"transcribestreaming",
"webassembly",
]
15 changes: 10 additions & 5 deletions aws/sdk/integration-tests/dynamodb/tests/movies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,36 @@ async fn create_table(client: &Client, table_name: &str) {
KeySchemaElement::builder()
.attribute_name("year")
.key_type(KeyType::Hash)
.build(),
.build()
.unwrap(),
Comment on lines +31 to +32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change build to try_build and add a build that panics?

)
.key_schema(
KeySchemaElement::builder()
.attribute_name("title")
.key_type(KeyType::Range)
.build(),
.build()
.unwrap(),
)
.attribute_definitions(
AttributeDefinition::builder()
.attribute_name("year")
.attribute_type(ScalarAttributeType::N)
.build(),
.build()
.unwrap(),
)
.attribute_definitions(
AttributeDefinition::builder()
.attribute_name("title")
.attribute_type(ScalarAttributeType::S)
.build(),
.build()
.unwrap(),
Comment on lines 48 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side note: I wonder if we want this API to accept impl TryInto<AttributeDefinitions>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put another way, what if users could pass a builder directly?

)
.provisioned_throughput(
ProvisionedThroughput::builder()
.read_capacity_units(10)
.write_capacity_units(10)
.build(),
.build()
.unwrap(),
)
.send()
.await
Expand Down
Loading
Loading