Clone-able structs

[Jacco]

Motivation and Context

Being able to clone some Fluent structs is sometimes very useful for reuse.

Fixes: awslabs/aws-sdk-rust#254

Description

If the Inner is Clone we add the Clone derive to the stuct (handle is also Clone)

Following currently you need to re-create the fluent stuct like in the example below:

fn author_query(client: &Client) -> aws_sdk_dynamodb::client::fluent_builders::Query {
    client
        .query()
        .table_name("Blog")
        .index_name("GSI1")
        .limit(1)
        .key_condition_expression("#key = :value".to_string())
        .expression_attribute_names("#key".to_string(), "SK".to_string())
        .expression_attribute_values(":value".to_string(), AttributeValue::S("USER".to_string()))
}

#[derive(Clone, Debug, Serialize, Deserialize)]
struct Author {
    id: String,
    name: String,
}

async fn get_authors(client: &Client) -> HashMap::<String, Author> {
    let mut last: Option<HashMap<String, AttributeValue>> = None;
    let mut result = HashMap::<String, Author>::new();
    loop {
        match author_query(&client)
            .set_exclusive_start_key(last)
            .send()
            .await {
                Ok(resp) => {
                    match &resp.items {
                        Some(recs) => {
                            for item in recs {
                                let auth = Author {
                                    id: item["PK"].as_s().ok().unwrap().to_string(),
                                    name: item["SRT"].as_s().ok().unwrap().to_string()
                                };
                                result.insert(auth.id.to_owned(), auth);
                            }
                        }
                        None => {

                        }
                    }
                    match resp.last_evaluated_key() {
                        Some(lev) => {
                            last = Some(lev.to_owned())
                        }
                        None => {
                            break;
                        }
                    }
                }
                Err(e) => {
                    println!("error {}", e);
                    break;
                } 
            }
    }
    return result;
}

This can improve to (not tested yet):

fn author_query(client: &Client) -> aws_sdk_dynamodb::client::fluent_builders::Query {
    client
        .query()
        .table_name("Blog")
        .index_name("GSI1")
        .limit(1)
        .key_condition_expression("#key = :value".to_string())
        .expression_attribute_names("#key".to_string(), "SK".to_string())
        .expression_attribute_values(":value".to_string(), AttributeValue::S("USER".to_string()))
}

#[derive(Clone, Debug, Serialize, Deserialize)]
struct Author {
    id: String,
    name: String,
}

let qry = author_query(&client); // <-- CONSTRUCT ONCE

async fn get_authors(client: &Client) -> HashMap::<String, Author> {
    let mut last: Option<HashMap<String, AttributeValue>> = None;
    let mut result = HashMap::<String, Author>::new();
    loop {
        match qry.clone() // <-- HERE IS THE CHANGE
            .set_exclusive_start_key(last)
            .send()
            .await {
                Ok(resp) => {
                    match &resp.items {
                        Some(recs) => {
                            for item in recs {
                                let auth = Author {
                                    id: item["PK"].as_s().ok().unwrap().to_string(),
                                    name: item["SRT"].as_s().ok().unwrap().to_string()
                                };
                                result.insert(auth.id.to_owned(), auth);
                            }
                        }
                        None => {

                        }
                    }
                    match resp.last_evaluated_key() {
                        Some(lev) => {
                            last = Some(lev.to_owned())
                        }
                        None => {
                            break;
                        }
                    }
                }
                Err(e) => {
                    println!("error {}", e);
                    break;
                } 
            }
    }
    return result;
}

Testing

For now I only ran
./gradlew :aws:sdk:assemble
./gradlew :aws:sdk:cargoCheck

Checklist

• I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
• 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.

[rcoh]

this looks great! Do you mind adding a changelog entry?

[Jacco]

@rcoh Added one for [[smithy-rs]]

[rcoh]

thanks! one more thing, can you also drop a test in here: https://github.com/awslabs/smithy-rs/blob/a46e4899cc854e02e2c25d20797a497c67ad28f4/aws/sdk/integration-tests/dynamodb/tests/ that utilizes the new clone functionality? That will ensure that this never regresses. Doesn't need to dispatch or request or anything, just utilize .clone() on the fluent builder.

[Jacco]

Will do. Something like this?

    let mut qry = movies_in_year(table_name, 2022).clone();
    let films_2013_again = client
    .call(
        qry
            .expression_attribute_values(":yyyy".to_string(), AttributeValue::N("2013".to_string()))
            .make_operation(&conf)
            .await
            .expect("valid request"),
    )
    .await
    .expect("query should succeed");
    assert_eq!(films_2013_again.count, 2);   
``

[rcoh]

yup, although I'd probably make a standalone test case so that it's a bit more clear why it's there. I think clippy will probably tell you that clone needs to be removed, then someone removes it etc. So we need to set up a slightly contrived example where the clone is actually load-bearing

[Jacco]

Would this be OK?

Still not entirely happy with the code because cloning is happening every loop iteration. Can this be prevented?

use std::collections::HashMap;
use aws_sdk_dynamodb::model::AttributeValue;
use aws_sdk_dynamodb::{Client, Error, client::fluent_builders::Query};

fn blog_query(client: &Client) -> Query {
    client
        .query()
        .table_name("Blog")
        .index_name("GSI1")
        .limit(1)
        .key_condition_expression("#key = :value".to_string())
        .expression_attribute_names("#key".to_string(), "SK".to_string())
        .expression_attribute_values(":value".to_string(), AttributeValue::S("POST".to_string()))
}

#[derive(Clone, Debug)]
struct Blog {
    id: String,
    created: String,
}

async fn get_blogs(query_template: Query) -> HashMap::<String, Blog> {
    let mut last: Option<HashMap<String, AttributeValue>> = None;
    let mut result = HashMap::<String, Blog>::new();
    loop {
        let query_result = query_template
            .to_owned() // <-- cloning is happening here
            .set_exclusive_start_key(last)
            .send()
            .await;
        match query_result {
                Ok(response) => {
                    if let Some(recs) = &response.items {
                        for item in recs {
                            let auth = Blog {
                                id: item["PK"].as_s().ok().unwrap().to_string(),
                                created: item["SRT"].as_s().ok().unwrap().to_string()
                            };
                            result.insert(auth.id.to_owned(), auth);
                        }
                    }
                    if let Some(last_key) = response.last_evaluated_key() {
                        last = Some(last_key.to_owned())
                    } else {
                        break;
                    }
                }
                Err(e) => {
                    println!("error {}", e);
                    break;
                } 
            }
    }
    return result;
}

#[tokio::main]
async fn main() -> Result<(), Error> {
    let shared_config = aws_config::load_from_env().await;
    let client = Client::new(&shared_config);

    // create a query template once
    let blog_query = blog_query(&client);

    let blogs = get_blogs(blog_query).await;

    println!("Blogs");
    for (key, value) in &blogs {
        println!("{}: {}", key, value.id);
    }
    Ok(())
}

[rcoh]

I might do something super simple and focused like this:

/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0.
 */

use aws_types::credentials::SharedCredentialsProvider;
use aws_types::region::Region;
use aws_types::Credentials;

// compiling this function validates that fluent builders are cloneable
#[allow(dead_code)]
async fn ensure_builders_clone() {
    let shared_config = aws_types::config::Config::builder()
        .region(Region::new("us-east-4"))
        .credentials_provider(SharedCredentialsProvider::new(Credentials::new(
            "asdf", "asdf", None, None, "test",
        )))
        .build();
    let client = aws_sdk_dynamodb::Client::new(&shared_config);
    let base_query = client.list_tables();
    let mut tables = vec![];
    for i in 0..100 {
        let query = base_query
            .clone()
            .exclusive_start_table_name(format!("table-{}", i));
        tables.extend(
            query
                .send()
                .await
                .expect("failed")
                .table_names
                .unwrap_or_default(),
        );
    }
}

[Jacco]

@rcoh added the test and also two Clone directives.

[rcoh]

LGTM!

[dcormier]

Thank you both!