add support for nullable struct members when generating AWS SDKs

CHANGELOG.next.toml

@@ -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)

aws/rust-runtime/aws-config/src/sts/util.rs

@@ -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,
     ))

aws/rust-runtime/aws-inlineable/src/route53_resource_id_preprocessor.rs

@@ -16,7 +16,7 @@ use std::marker::PhantomData;
 // This function is only used to strip prefixes from resource IDs at the time they're passed as
 // input to a request. Resource IDs returned in responses may or may not include a prefix.
 /// Strip the resource type prefix from resource ID return
-fn trim_resource_id(resource_id: &mut Option<String>) {
+fn trim_resource_id(resource_id: &mut String) {
     const PREFIXES: &[&str] = &[
         "/hostedzone/",
         "hostedzone/",
@@ -27,28 +27,24 @@ fn trim_resource_id(resource_id: &mut Option<String>) {
     ];
 
     for prefix in PREFIXES {
-        if let Some(id) = resource_id
-            .as_deref()
-            .unwrap_or_default()
-            .strip_prefix(prefix)
-        {
-            *resource_id = Some(id.to_string());
+        if let Some(trimmed_id) = resource_id.strip_prefix(prefix) {
+            *resource_id = trimmed_id.to_string();
             return;
         }
     }
 }
 
 pub(crate) struct Route53ResourceIdInterceptor<G, T>
 where
-    G: for<'a> Fn(&'a mut T) -> &'a mut Option<String>,
+    G: for<'a> Fn(&'a mut T) -> Option<&'a mut String>,
 {
     get_mut_resource_id: G,
     _phantom: PhantomData<T>,
 }
 
 impl<G, T> fmt::Debug for Route53ResourceIdInterceptor<G, T>
 where
-    G: for<'a> Fn(&'a mut T) -> &'a mut Option<String>,
+    G: for<'a> Fn(&'a mut T) -> Option<&'a mut String>,
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Route53ResourceIdInterceptor").finish()
@@ -57,7 +53,7 @@ where
 
 impl<G, T> Route53ResourceIdInterceptor<G, T>
 where
-    G: for<'a> Fn(&'a mut T) -> &'a mut Option<String>,
+    G: for<'a> Fn(&'a mut T) -> Option<&'a mut String>,
 {
     pub(crate) fn new(get_mut_resource_id: G) -> Self {
         Self {
@@ -69,7 +65,7 @@ where
 
 impl<G, T> Interceptor for Route53ResourceIdInterceptor<G, T>
 where
-    G: for<'a> Fn(&'a mut T) -> &'a mut Option<String> + Send + Sync,
+    G: for<'a> Fn(&'a mut T) -> Option<&'a mut String> + Send + Sync,
     T: fmt::Debug + Send + Sync + 'static,
 {
     fn name(&self) -> &'static str {
@@ -83,8 +79,9 @@ where
         _cfg: &mut ConfigBag,
     ) -> Result<(), BoxError> {
         let input: &mut T = context.input_mut().downcast_mut().expect("correct type");
-        let field = (self.get_mut_resource_id)(input);
-        trim_resource_id(field);
+        if let Some(field) = (self.get_mut_resource_id)(input) {
+            trim_resource_id(field)
+        }
         Ok(())
     }
 }
@@ -96,48 +93,39 @@ mod test {
     #[test]
     fn does_not_change_regular_zones() {
         struct OperationInput {
-            resource: Option<String>,
+            resource: String,
         }
 
         let mut operation = OperationInput {
-            resource: Some("Z0441723226OZ66S5ZCNZ".to_string()),
+            resource: "Z0441723226OZ66S5ZCNZ".to_string(),
         };
         trim_resource_id(&mut operation.resource);
-        assert_eq!(
-            &operation.resource.unwrap_or_default(),
-            "Z0441723226OZ66S5ZCNZ"
-        );
+        assert_eq!(&operation.resource, "Z0441723226OZ66S5ZCNZ");
     }
 
     #[test]
     fn sanitizes_prefixed_zone() {
         struct OperationInput {
-            change_id: Option<String>,
+            change_id: String,
         }
 
         let mut operation = OperationInput {
-            change_id: Some("/change/Z0441723226OZ66S5ZCNZ".to_string()),
+            change_id: "/change/Z0441723226OZ66S5ZCNZ".to_string(),
         };
         trim_resource_id(&mut operation.change_id);
-        assert_eq!(
-            &operation.change_id.unwrap_or_default(),
-            "Z0441723226OZ66S5ZCNZ"
-        );
+        assert_eq!(&operation.change_id, "Z0441723226OZ66S5ZCNZ");
     }
 
     #[test]
     fn allow_no_leading_slash() {
         struct OperationInput {
-            hosted_zone: Option<String>,
+            hosted_zone: String,
         }
 
         let mut operation = OperationInput {
-            hosted_zone: Some("hostedzone/Z0441723226OZ66S5ZCNZ".to_string()),
+            hosted_zone: "hostedzone/Z0441723226OZ66S5ZCNZ".to_string(),
         };
         trim_resource_id(&mut operation.hosted_zone);
-        assert_eq!(
-            &operation.hosted_zone.unwrap_or_default(),
-            "Z0441723226OZ66S5ZCNZ"
-        );
+        assert_eq!(&operation.hosted_zone, "Z0441723226OZ66S5ZCNZ");
     }
 }

aws/sdk-adhoc-test/build.gradle.kts

@@ -37,6 +37,8 @@ dependencies {
     implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
 }
 
+fun getNullabilityCheckMode(): String = properties.get("nullability.check.mode") ?: "CLIENT_CAREFUL"
+
 val allCodegenTests = listOf(
     CodegenTest(
         "com.amazonaws.apigateway#BackplaneControlService",
@@ -45,7 +47,8 @@ val allCodegenTests = listOf(
         extraConfig = """
             ,
             "codegen": {
-                "includeFluentClient": false
+                "includeFluentClient": false,
+                "nullabilityCheckMode": ${getNullabilityCheckMode()}
             },
             "customizationConfig": {
                 "awsSdk": {
@@ -61,7 +64,8 @@ val allCodegenTests = listOf(
         extraConfig = """
             ,
             "codegen": {
-                "includeFluentClient": false
+                "includeFluentClient": false,
+                "nullabilityCheckMode": ${getNullabilityCheckMode()}
             },
             "customizationConfig": {
                 "awsSdk": {

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/route53/Route53Decorator.kt

@@ -80,10 +80,11 @@ class TrimResourceIdCustomization(
                         RuntimeType.forInlineDependency(
                             InlineAwsDependency.forRustFile("route53_resource_id_preprocessor"),
                         ).resolve("Route53ResourceIdInterceptor")
+
                     rustTemplate(
                         """
                         #{Route53ResourceIdInterceptor}::new(|input: &mut #{Input}| {
-                            &mut input.$fieldName
+                            input.$fieldName.as_mut()
                         })
                         """,
                         "Input" to codegenContext.symbolProvider.toSymbol(inputShape),

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/timestream/TimestreamDecorator.kt

@@ -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,
                     ))

aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/customize/ec2/EC2MakePrimitivesOptionalTest.kt

@@ -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 {
@@ -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
         }
     }
 }

aws/sdk/build.gradle.kts

@@ -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) }
@@ -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",

aws/sdk/integration-tests/Cargo.toml

@@ -14,7 +14,7 @@ members = [
     "s3",
     "s3control",
     "sts",
-    "transcribestreaming",
     "timestreamquery",
+    "transcribestreaming",
     "webassembly",
 ]

aws/sdk/integration-tests/dynamodb/tests/movies.rs

@@ -28,25 +28,29 @@ async fn create_table(client: &Client, table_name: &str) {
             KeySchemaElement::builder()
                 .attribute_name("year")
                 .key_type(KeyType::Hash)
-                .build(),
+                .build()
+                .unwrap(),
         )
         .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(),
         )
         .provisioned_throughput(
             ProvisionedThroughput::builder()

aws/sdk/integration-tests/qldbsession/tests/integration.rs

@@ -3,19 +3,14 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-use aws_sdk_qldbsession as qldbsession;
+use aws_sdk_qldbsession::config::{Config, Credentials, Region};
+use aws_sdk_qldbsession::types::StartSessionRequest;
+use aws_sdk_qldbsession::Client;
 use aws_smithy_client::test_connection::TestConnection;
 use aws_smithy_http::body::SdkBody;
 use http::Uri;
-use qldbsession::config::{Config, Credentials, Region};
-use qldbsession::types::StartSessionRequest;
-use qldbsession::Client;
 use std::time::{Duration, UNIX_EPOCH};
 
-// TODO(DVR): having the full HTTP requests right in the code is a bit gross, consider something
-// like https://github.com/davidbarsky/sigv4/blob/master/aws-sigv4/src/lib.rs#L283-L315 to store
-// the requests/responses externally
-
 #[tokio::test]
 async fn signv4_use_correct_service_name() {
     let conn = TestConnection::new(vec![(
@@ -46,7 +41,8 @@ async fn signv4_use_correct_service_name() {
         .start_session(
             StartSessionRequest::builder()
                 .ledger_name("not-real-ledger")
-                .build(),
+                .build()
+                .unwrap(),
         )
         .customize()
         .await

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientCodegenVisitor.kt

@@ -7,7 +7,6 @@ package software.amazon.smithy.rust.codegen.client.smithy
 
 import software.amazon.smithy.build.PluginContext
 import software.amazon.smithy.model.Model
-import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.OperationShape
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
@@ -77,10 +76,11 @@ class ClientCodegenVisitor(
         val rustSymbolProviderConfig = RustSymbolProviderConfig(
             runtimeConfig = settings.runtimeConfig,
             renameExceptions = settings.codegenConfig.renameExceptions,
-            nullabilityCheckMode = NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1,
+            nullabilityCheckMode = settings.codegenConfig.nullabilityCheckMode,
             moduleProvider = ClientModuleProvider,
             nameBuilderFor = { symbol -> "${symbol.name}Builder" },
         )
+
         val baseModel = baselineTransform(context.model)
         val untransformedService = settings.getService(baseModel)
         val (protocol, generator) = ClientProtocolLoader(