Add integration tests for uploading multiple objects

aws-s3-transfer-manager/Cargo.toml

@@ -36,12 +36,13 @@ clap = { version = "4.5.7", default-features = false, features = ["derive", "std
 console-subscriber = "0.4.0"
 http-02x = { package = "http", version = "0.2.9" }
 http-body-1x = { package = "http-body", version = "1" }
-tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
-tempfile = "3.12.0"
 fastrand = "2.1.1"
 futures-test = "0.3.30"
-tower-test = "0.4.0"
+tempfile = "3.12.0"
+test-common = { path = "./test-common" }
 tokio-test = "0.4.4"
+tower-test = "0.4.0"
+tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
 
 [target.'cfg(not(target_env = "msvc"))'.dev-dependencies]
 jemallocator = "0.5.4"

aws-s3-transfer-manager/src/error.rs

@@ -21,7 +21,7 @@ pub struct Error {
 }
 
 /// General categories of transfer errors.
-#[derive(Debug, Clone)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 #[non_exhaustive]
 pub enum ErrorKind {
     /// Operation input validation issues

aws-s3-transfer-manager/src/operation/upload_objects/input.rs

@@ -8,8 +8,6 @@ use aws_smithy_types::error::operation::BuildError;
 
 use std::path::{Path, PathBuf};
 
-// TODO - docs and examples on the interaction of key_prefix & delimiter
-
 /// Input type for uploading multiple objects
 #[non_exhaustive]
 #[derive(Clone, Debug)]

aws-s3-transfer-manager/src/operation/upload_objects/output.rs

@@ -36,7 +36,7 @@ impl UploadObjectsOutput {
         self.failed_transfers.as_deref().unwrap_or_default()
     }
 
-    /// The number of bytes successfully transferred (downloaded)
+    /// The number of bytes successfully transferred (uploaded)
     pub fn total_bytes_transferred(&self) -> u64 {
         self.total_bytes_transferred
     }

aws-s3-transfer-manager/src/operation/upload_objects/worker.rs

@@ -235,9 +235,7 @@ mod tests {
     use crate::operation::upload_objects::UploadObjectsInputBuilder;
     use std::collections::BTreeMap;
     use std::error::Error as _;
-    use std::fs;
-    use std::io::Write;
-    use tempfile::{tempdir, TempDir};
+    use test_common::create_test_dir;
 
     #[cfg(target_family = "unix")]
     #[test]
@@ -302,42 +300,6 @@ mod tests {
         );
     }
 
-    #[cfg(target_family = "unix")]
-    fn create_test_dir(
-        recursion_root: Option<&str>,
-        files: Vec<(&str, usize)>,
-        inaccessible_dir_relative_paths: &[&str],
-    ) -> TempDir {
-        let temp_dir = match recursion_root {
-            Some(root) => TempDir::with_prefix(root).unwrap(),
-            None => tempdir().unwrap(),
-        };
-
-        // Create the directory structure and files
-        for (path, size) in files {
-            let full_path = temp_dir.path().join(path);
-            let parent = full_path.parent().unwrap();
-
-            // Create the parent directories if they don't exist
-            fs::create_dir_all(parent).unwrap();
-
-            // Create the .jpg file with the specified size
-            let mut file = fs::File::create(&full_path).unwrap();
-            file.write_all(&vec![0; size]).unwrap(); // Writing `size` byte
-        }
-
-        // Set the directories in `inaccessible_dir_relative_paths` to be inaccessible,
-        // which will in turn render the files within those directories inaccessible
-        for dir_relative_path in inaccessible_dir_relative_paths {
-            let dir_path = temp_dir.path().join(*dir_relative_path);
-            let mut permissions = fs::metadata(&dir_path).unwrap().permissions();
-            std::os::unix::fs::PermissionsExt::set_mode(&mut permissions, 0o000); // No permissions for anyone
-            fs::set_permissions(dir_path, permissions).unwrap();
-        }
-
-        temp_dir
-    }
-
     async fn exercise_list_directory_contents(
         input: UploadObjectsInput,
     ) -> (BTreeMap<String, usize>, Vec<error::Error>) {

aws-s3-transfer-manager/test-common/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "test-common"
+version = "0.1.0"
+edition = "2021"
+publish = false
+
+[dependencies]
+tempfile = "3.12.0"

aws-s3-transfer-manager/test-common/src/lib.rs

@@ -0,0 +1,50 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#![cfg(target_family = "unix")]
+
+use std::{fs, io::Write};
+use tempfile::{tempdir, TempDir};
+
+/// Create a directory structure rooted at `recursion_root`, containing files with sizes
+/// specified in `files`
+/// 
+/// For testing purposes, certain directories (and all files within them) can be made 
+/// inaccessible by providing `inaccessible_dir_relative_paths`, which should be relative 
+/// to `recursion_root`.
+pub fn create_test_dir(
+    recursion_root: Option<&str>,
+    files: Vec<(&str, usize)>,
+    inaccessible_dir_relative_paths: &[&str],
+) -> TempDir {
+    let temp_dir = match recursion_root {
+        Some(root) => TempDir::with_prefix(root).unwrap(),
+        None => tempdir().unwrap(),
+    };
+
+    // Create the directory structure and files
+    for (path, size) in files {
+        let full_path = temp_dir.path().join(path);
+        let parent = full_path.parent().unwrap();
+
+        // Create the parent directories if they don't exist
+        fs::create_dir_all(parent).unwrap();
+
+        // Create the .jpg file with the specified size
+        let mut file = fs::File::create(&full_path).unwrap();
+        file.write_all(&vec![0; size]).unwrap(); // Writing `size` byte
+    }
+
+    // Set the directories in `inaccessible_dir_relative_paths` to be inaccessible,
+    // which will in turn render the files within those directories inaccessible
+    for dir_relative_path in inaccessible_dir_relative_paths {
+        let dir_path = temp_dir.path().join(*dir_relative_path);
+        let mut permissions = fs::metadata(&dir_path).unwrap().permissions();
+        std::os::unix::fs::PermissionsExt::set_mode(&mut permissions, 0o000); // No permissions for anyone
+        fs::set_permissions(dir_path, permissions).unwrap();
+    }
+
+    temp_dir
+}

aws-s3-transfer-manager/tests/download_objects_test.rs

@@ -401,5 +401,5 @@ async fn test_destination_dir_not_valid() {
         .unwrap_err();
 
     let err_str = format!("{}", DisplayErrorContext(err));
-    assert!(err_str.contains("is not a directory"));
+    assert!(err_str.contains("target is not a directory"));
 }

aws-s3-transfer-manager/tests/upload_objects_test.rs

@@ -0,0 +1,191 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#![cfg(target_family = "unix")]
+
+use aws_s3_transfer_manager::{error::ErrorKind, types::FailedTransferPolicy};
+use aws_sdk_s3::{
+    error::DisplayErrorContext,
+    operation::{
+        complete_multipart_upload::CompleteMultipartUploadOutput,
+        create_multipart_upload::CreateMultipartUploadOutput, upload_part::UploadPartOutput,
+    },
+    Client,
+};
+use aws_smithy_mocks_experimental::{mock, mock_client, RuleMode};
+use test_common::create_test_dir;
+
+// Create an S3 client with mock behavior configured
+//
+// We intentionally avoid being specific about the expected input and output for mocks,
+// as long as the execution of uploading multiple objects completes successfully.
+// Setting expectations that are too precise can lead to brittle tests.
+fn mock_s3_client(bucket_name: String) -> Client {
+    let upload_id = "test-upload-id".to_owned();
+
+    let create_mpu = mock!(aws_sdk_s3::Client::create_multipart_upload).then_output({
+        let upload_id = upload_id.clone();
+        move || {
+            CreateMultipartUploadOutput::builder()
+                .upload_id(upload_id.clone())
+                .build()
+        }
+    });
+
+    let upload_part = mock!(aws_sdk_s3::Client::upload_part)
+        .match_requests({
+            let upload_id = upload_id.clone();
+            move |input| {
+                input.upload_id.as_ref() == Some(&upload_id) && input.bucket() == Some(&bucket_name)
+            }
+        })
+        .then_output(|| UploadPartOutput::builder().build());
+
+    let complete_mpu = mock!(aws_sdk_s3::Client::complete_multipart_upload)
+        .match_requests({
+            let upload_id = upload_id.clone();
+            move |r| r.upload_id.as_ref() == Some(&upload_id)
+        })
+        .then_output(|| CompleteMultipartUploadOutput::builder().build());
+
+    mock_client!(
+        aws_sdk_s3,
+        RuleMode::MatchAny,
+        &[create_mpu, upload_part, complete_mpu]
+    )
+}
+
+#[tokio::test]
+async fn test_successful_multiple_objects_upload() {
+    let recursion_root = "test";
+    let files = vec![
+        ("sample.jpg", 1),
+        ("photos/2022/January/sample.jpg", 1),
+        ("photos/2022/February/sample1.jpg", 1),
+        ("photos/2022/February/sample2.jpg", 1),
+        ("photos/2022/February/sample3.jpg", 1),
+    ];
+    let test_dir = create_test_dir(Some(&recursion_root), files.clone(), &[]);
+
+    let bucket_name = "test-bucket";
+    let config = aws_s3_transfer_manager::Config::builder()
+        .client(mock_s3_client(bucket_name.to_owned()))
+        .build();
+    let sut = aws_s3_transfer_manager::Client::new(config);
+
+    let handle = sut
+        .upload_objects()
+        .bucket(bucket_name)
+        .source(test_dir.path())
+        .recursive(true)
+        .send()
+        .await
+        .unwrap();
+
+    let output = handle.join().await.unwrap();
+    assert_eq!(5, output.objects_uploaded());
+    assert!(output.failed_transfers().is_empty());
+    assert_eq!(5, output.total_bytes_transferred());
+}
+
+#[tokio::test]
+async fn test_failed_upload_policy_continue() {
+    let recursion_root = "test";
+    let files = vec![
+        ("sample.jpg", 1),
+        ("photos/2022/January/sample.jpg", 1),
+        ("photos/2022/February/sample1.jpg", 1),
+        ("photos/2022/February/sample2.jpg", 1),
+        ("photos/2022/February/sample3.jpg", 1),
+    ];
+    // Make all files inaccessible under `photos/2022/February`
+    let inaccessible_dir_relative_path = "photos/2022/February";
+    let test_dir = create_test_dir(
+        Some(&recursion_root),
+        files.clone(),
+        &[inaccessible_dir_relative_path],
+    );
+
+    let bucket_name = "test-bucket";
+    let config = aws_s3_transfer_manager::Config::builder()
+        .client(mock_s3_client(bucket_name.to_owned()))
+        .build();
+    let sut = aws_s3_transfer_manager::Client::new(config);
+
+    let handle = sut
+        .upload_objects()
+        .bucket(bucket_name)
+        .source(test_dir.path())
+        .recursive(true)
+        .failure_policy(FailedTransferPolicy::Continue)
+        .send()
+        .await
+        .unwrap();
+
+    let output = handle.join().await.unwrap();
+    assert_eq!(2, output.objects_uploaded());
+    assert_eq!(1, output.failed_transfers().len()); // Cannot traverse inaccessible dir to count how many files are in it
+    assert_eq!(2, output.total_bytes_transferred());
+}
+
+/// Fail when source is not a directory
+#[tokio::test]
+async fn test_source_dir_not_valid() {
+    let source = tempfile::NamedTempFile::new().unwrap();
+
+    let bucket_name = "test-bucket";
+    let config = aws_s3_transfer_manager::Config::builder()
+        .client(mock_s3_client(bucket_name.to_owned()))
+        .build();
+    let sut = aws_s3_transfer_manager::Client::new(config);
+
+    let err = sut
+        .upload_objects()
+        .bucket(bucket_name)
+        .source(source.path())
+        .send()
+        .await
+        .unwrap_err();
+
+    assert_eq!(&ErrorKind::InputInvalid, err.kind());
+    assert!(format!("{}", DisplayErrorContext(err)).contains("is not a directory"));
+}
+
+#[tokio::test]
+async fn test_error_when_custom_delimiter_appears_in_filename() {
+    let recursion_root = "test";
+    let files = vec![
+        ("sample.jpg", 1),
+        ("photos/2022-January/sample.jpg", 1),
+        ("photos/2022-February/sample1.jpg", 1),
+        ("photos/2022-February/sample2.jpg", 1),
+        ("photos/2022-February/sample3.jpg", 1),
+    ];
+    let test_dir = create_test_dir(Some(&recursion_root), files.clone(), &[]);
+
+    let bucket_name = "test-bucket";
+    let config = aws_s3_transfer_manager::Config::builder()
+        .client(mock_s3_client(bucket_name.to_owned()))
+        .build();
+    let sut = aws_s3_transfer_manager::Client::new(config);
+
+    // Getting `handle` is ok, i.e., tasks should be spawned from `UploadObjects::orchestrate`
+    let handle = sut
+        .upload_objects()
+        .bucket(bucket_name)
+        .source(test_dir.path())
+        .recursive(true)
+        .delimiter("-")
+        .send()
+        .await
+        .unwrap();
+
+    // But unwrapping it should fail
+    let err = handle.join().await.unwrap_err();
+
+    assert_eq!(&ErrorKind::InputInvalid, err.kind());
+    assert!(format!("{}", DisplayErrorContext(err))
+        .contains("a custom delimiter `-` should not appear"));
+}