From a59f4948746d40932fce5369a5878d5978e07446 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Tue, 29 Apr 2025 19:17:25 +0800 Subject: [PATCH 1/2] refactor!: Change FileIO::remove_all to remove_dir_all Signed-off-by: Xuanwo --- crates/iceberg/src/io/file_io.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/io/file_io.rs b/crates/iceberg/src/io/file_io.rs index cadef7d54..ade46e265 100644 --- a/crates/iceberg/src/io/file_io.rs +++ b/crates/iceberg/src/io/file_io.rs @@ -95,9 +95,14 @@ impl FileIO { /// # Arguments /// /// * path: It should be *absolute* path starting with scheme string used to construct [`FileIO`]. - pub async fn remove_all(&self, path: impl AsRef) -> Result<()> { + pub async fn remove_dir_all(&self, path: impl AsRef) -> Result<()> { let (op, relative_path) = self.inner.create_operator(&path)?; - Ok(op.remove_all(relative_path).await?) + let path = if relative_path.ends_with('/') { + relative_path.to_string() + } else { + format!("{relative_path}/") + }; + Ok(op.remove_all(&path).await?) } /// Check file exists. @@ -441,7 +446,7 @@ mod tests { let file_io = create_local_file_io(); assert!(file_io.exists(&a_path).await.unwrap()); - file_io.remove_all(&sub_dir_path).await.unwrap(); + file_io.remove_dir_all(&sub_dir_path).await.unwrap(); assert!(!file_io.exists(&b_path).await.unwrap()); assert!(!file_io.exists(&c_path).await.unwrap()); assert!(file_io.exists(&a_path).await.unwrap()); @@ -460,7 +465,7 @@ mod tests { let file_io = create_local_file_io(); assert!(!file_io.exists(&full_path).await.unwrap()); assert!(file_io.delete(&full_path).await.is_ok()); - assert!(file_io.remove_all(&full_path).await.is_ok()); + assert!(file_io.remove_dir_all(&full_path).await.is_ok()); } #[tokio::test] From 4cec26093035a64d9bc4a7279a9375bc6ecf6a8e Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Wed, 14 May 2025 17:41:15 +0800 Subject: [PATCH 2/2] Address comments Signed-off-by: Xuanwo --- crates/iceberg/src/io/file_io.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/iceberg/src/io/file_io.rs b/crates/iceberg/src/io/file_io.rs index 3c23064dc..7557883b0 100644 --- a/crates/iceberg/src/io/file_io.rs +++ b/crates/iceberg/src/io/file_io.rs @@ -95,6 +95,23 @@ impl FileIO { /// # Arguments /// /// * path: It should be *absolute* path starting with scheme string used to construct [`FileIO`]. + #[deprecated(note = "use remove_dir_all instead", since = "0.4.0")] + pub async fn remove_all(&self, path: impl AsRef) -> Result<()> { + let (op, relative_path) = self.inner.create_operator(&path)?; + Ok(op.remove_all(relative_path).await?) + } + + /// Remove the path and all nested dirs and files recursively. + /// + /// # Arguments + /// + /// * path: It should be *absolute* path starting with scheme string used to construct [`FileIO`]. + /// + /// # Behavior + /// + /// - If the path is a file or not exist, this function will be no-op. + /// - If the path is a empty directory, this function will remove the directory itself. + /// - If the path is a non-empty directory, this function will remove the directory and all nested files and directories. pub async fn remove_dir_all(&self, path: impl AsRef) -> Result<()> { let (op, relative_path) = self.inner.create_operator(&path)?; let path = if relative_path.ends_with('/') { @@ -446,6 +463,14 @@ mod tests { let file_io = create_local_file_io(); assert!(file_io.exists(&a_path).await.unwrap()); + // Remove a file should be no-op. + file_io.remove_dir_all(&a_path).await.unwrap(); + assert!(file_io.exists(&a_path).await.unwrap()); + + // Remove a not exist dir should be no-op. + file_io.remove_dir_all("not_exists/").await.unwrap(); + + // Remove a dir should remove all files in it. file_io.remove_dir_all(&sub_dir_path).await.unwrap(); assert!(!file_io.exists(&b_path).await.unwrap()); assert!(!file_io.exists(&c_path).await.unwrap());