Unverified Commit aaf71c83 authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

Produce `&[T]` instead of `Option<&[T]>` for list fields (#2995) 

## Motivation and Context
When the returned field is a list, there is _almost_ never a reason to
know if the returned value was null or `[]` — this addresses the 99%
case.

Before:
```rust
fn blah(&self) -> Option<&[T]> { self.blah.as_deref() }
```

After:
```rust
fn blah(&self) -> &[T] { self.blah.as_deref().unwrap_or_default() }
```

**note**: no impact on servers by default, see codegen diff.

## Description
Update accessors for lists.

## Testing
- [x] codegen diff audit (no server changes)
- [x] unit tests

## 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
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK 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._
parent 3229089f

CHANGELOG.next.toml

+22 −0
Original line number Diff line number Diff line
@@ -178,6 +178,28 @@ references = ["smithy-rs#2985"] 
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }

author = "rcoh"



[[smithy-rs]]

message = """

Structure members with the type `Option<Vec<T>>` now produce an accessor with the type `&[T]` instead of `Option<&[T]>`. This is enabled by default for clients and can be disabled by updating your smithy-build.json with the following setting:

```json

{

  "codegen": {

    "flattenCollectionAccessors": false,

    ...

  }

}

```

"""

references = ["smithy-rs#2995"]

author = "rcoh"

meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all" }



[[aws-sdk-rust]]

message = "Structure members with the type `Option<Vec<T>>` now produce an accessor with the type `&[T]` instead of `Option<&[T]>`. To determine if the field was actually set use `.<field_name>.is_some()`."

references = ["smithy-rs#2995"]

author = "rcoh"

meta = { "breaking" = true, "tada" = false, "bug" = false }



[[smithy-rs]]

message = "Produce better docs when items are marked @required"

references = ["smithy-rs#2996"]

aws/sdk/integration-tests/dynamodb/tests/retries-with-client-rate-limiting.rs

+4 −4
Original line number Diff line number Diff line
@@ -86,14 +86,14 @@ async fn test_adaptive_retries_with_no_throttling_errors() { 
    let client = aws_sdk_dynamodb::Client::from_conf(config.clone());

    let res = client.list_tables().send().await.unwrap();

    assert_eq!(sleep_impl.total_duration(), Duration::from_secs(3));

    assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));

    assert_eq!(res.table_names(), expected_table_names.as_slice());

    // Three requests should have been made, two failing & one success

    assert_eq!(conn.requests().len(), 3);



    let client = aws_sdk_dynamodb::Client::from_conf(config.clone());

    let res = client.list_tables().send().await.unwrap();

    assert_eq!(sleep_impl.total_duration(), Duration::from_secs(3 + 1));

    assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));

    assert_eq!(res.table_names(), expected_table_names.as_slice());

    // Two requests should have been made, one failing & one success (plus previous requests)

    assert_eq!(conn.requests().len(), 5);
@@ -141,7 +141,7 @@ async fn test_adaptive_retries_with_throttling_errors() { 
    let client = aws_sdk_dynamodb::Client::from_conf(config.clone());

    let res = client.list_tables().send().await.unwrap();

    assert_eq!(sleep_impl.total_duration(), Duration::from_secs(40));

    assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));

    assert_eq!(res.table_names(), expected_table_names.as_slice());

    // Three requests should have been made, two failing & one success

    assert_eq!(conn.requests().len(), 3);
@@ -149,7 +149,7 @@ async fn test_adaptive_retries_with_throttling_errors() { 
    let res = client.list_tables().send().await.unwrap();

    assert!(Duration::from_secs(48) < sleep_impl.total_duration());

    assert!(Duration::from_secs(49) > sleep_impl.total_duration());

    assert_eq!(res.table_names(), Some(expected_table_names.as_slice()));

    assert_eq!(res.table_names(), expected_table_names.as_slice());

    // Two requests should have been made, one failing & one success (plus previous requests)

    assert_eq!(conn.requests().len(), 5);

}

aws/sdk/integration-tests/webassembly/src/list_buckets.rs

+1 −1
Original line number Diff line number Diff line
@@ -11,7 +11,7 @@ pub async fn s3_list_buckets() { 
    let shared_config = get_default_config().await;

    let client = Client::new(&shared_config);

    let result = client.list_buckets().send().await.unwrap();

    assert_eq!(result.buckets().unwrap().len(), 2)

    assert_eq!(result.buckets().len(), 2)

}



#[tokio::test]

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

+2 −0
Original line number Diff line number Diff line
@@ -223,6 +223,7 @@ class ClientCodegenVisitor( 
                        this,

                        shape,

                        codegenDecorator.structureCustomizations(codegenContext, emptyList()),

                        structSettings = codegenContext.structSettings(),

                    ).render()



                    implBlock(symbolProvider.toSymbol(shape)) {
@@ -246,6 +247,7 @@ class ClientCodegenVisitor( 
                    shape,

                    errorTrait,

                    codegenDecorator.errorImplCustomizations(codegenContext, emptyList()),

                    codegenContext.structSettings(),

                )

                errorGenerator::renderStruct to errorGenerator::renderBuilder

            }

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

+6 −1
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ data class ClientRustSettings( 
data class ClientCodegenConfig(

    override val formatTimeoutSeconds: Int = defaultFormatTimeoutSeconds,

    override val debugMode: Boolean = defaultDebugMode,

    override val flattenCollectionAccessors: Boolean = defaultFlattenAccessors,

    val nullabilityCheckMode: NullableIndex.CheckMode = NullableIndex.CheckMode.CLIENT,

    val renameExceptions: Boolean = defaultRenameExceptions,

    val includeFluentClient: Boolean = defaultIncludeFluentClient,
@@ -92,7 +93,7 @@ data class ClientCodegenConfig( 
    val includeEndpointUrlConfig: Boolean = defaultIncludeEndpointUrlConfig,

    val enableUserConfigurableRuntimePlugins: Boolean = defaultEnableUserConfigurableRuntimePlugins,

) : CoreCodegenConfig(

    formatTimeoutSeconds, debugMode,

    formatTimeoutSeconds, debugMode, defaultFlattenAccessors,

) {

    companion object {

        private const val defaultRenameExceptions = true
@@ -103,10 +104,14 @@ data class ClientCodegenConfig( 
        private const val defaultEnableUserConfigurableRuntimePlugins = true

        private const val defaultNullabilityCheckMode = "CLIENT"



        // Note: only clients default to true, servers default to false

        private const val defaultFlattenAccessors = true



        fun fromCodegenConfigAndNode(coreCodegenConfig: CoreCodegenConfig, node: Optional<ObjectNode>) =

            if (node.isPresent) {

                ClientCodegenConfig(

                    formatTimeoutSeconds = coreCodegenConfig.formatTimeoutSeconds,

                    flattenCollectionAccessors = node.get().getBooleanMemberOrDefault("flattenCollectionAccessors", defaultFlattenAccessors),

                    debugMode = coreCodegenConfig.debugMode,

                    eventStreamAllowList = node.get().getArrayMember("eventStreamAllowList").map { array ->

                        array.toList().mapNotNull { node ->