From c9d26c6e884e20bf0a758ae648cae2e9ef2fb449 Mon Sep 17 00:00:00 2001 From: weisd Date: Wed, 3 Sep 2025 15:12:58 +0800 Subject: [PATCH] Fix/delete version (#484) * fix:delete_version * fix:test_lifecycle_expiry_basic --------- Co-authored-by: likewu --- .../ahm/tests/lifecycle_integration_test.rs | 12 ++-- crates/filemeta/src/filemeta.rs | 66 ++++++++++++------- rustfs/src/admin/handlers/event.rs | 2 +- rustfs/src/server/http.rs | 4 +- 4 files changed, 50 insertions(+), 34 deletions(-) diff --git a/crates/ahm/tests/lifecycle_integration_test.rs b/crates/ahm/tests/lifecycle_integration_test.rs index 6138b671..8c74bb3d 100644 --- a/crates/ahm/tests/lifecycle_integration_test.rs +++ b/crates/ahm/tests/lifecycle_integration_test.rs @@ -29,8 +29,8 @@ use std::sync::OnceLock; use std::{path::PathBuf, sync::Arc, time::Duration}; use tokio::fs; use tokio::sync::RwLock; -use tracing::info; use tracing::warn; +use tracing::{debug, info}; static GLOBAL_ENV: OnceLock<(Vec, Arc)> = OnceLock::new(); static INIT: Once = Once::new(); @@ -278,7 +278,7 @@ async fn object_exists(ecstore: &Arc, bucket: &str, object: &str) -> bo #[allow(dead_code)] async fn object_is_delete_marker(ecstore: &Arc, bucket: &str, object: &str) -> bool { if let Ok(oi) = (**ecstore).get_object_info(bucket, object, &ObjectOptions::default()).await { - println!("oi: {:?}", oi); + debug!("oi: {:?}", oi); oi.delete_marker } else { panic!("object_is_delete_marker is error"); @@ -358,8 +358,10 @@ async fn test_lifecycle_expiry_basic() { let check_result = object_exists(&ecstore, bucket_name, object_name).await; println!("Object is_delete_marker after lifecycle processing: {check_result}"); - if !check_result { + if check_result { println!("❌ Object was not deleted by lifecycle processing"); + } else { + println!("✅ Object was successfully deleted by lifecycle processing"); // Let's try to get object info to see its details match ecstore .get_object_info(bucket_name, object_name, &rustfs_ecstore::store_api::ObjectOptions::default()) @@ -375,11 +377,9 @@ async fn test_lifecycle_expiry_basic() { println!("Error getting object info: {e:?}"); } } - } else { - println!("✅ Object was successfully deleted by lifecycle processing"); } - assert!(check_result); + assert!(!check_result); println!("✅ Object successfully expired"); // Stop scanner diff --git a/crates/filemeta/src/filemeta.rs b/crates/filemeta/src/filemeta.rs index dd530263..e0a2f924 100644 --- a/crates/filemeta/src/filemeta.rs +++ b/crates/filemeta/src/filemeta.rs @@ -578,45 +578,61 @@ impl FileMeta { } } + let mut found_index = None; for (i, version) in self.versions.iter().enumerate() { - if version.header.version_type != VersionType::Object || version.header.version_id != fi.version_id { - continue; - } - - let mut ver = self.get_idx(i)?; - - if fi.expire_restored { - ver.object.as_mut().unwrap().remove_restore_hdrs(); - let _ = self.set_idx(i, ver.clone()); - } else if fi.transition_status == TRANSITION_COMPLETE { - ver.object.as_mut().unwrap().set_transition(fi); - ver.object.as_mut().unwrap().reset_inline_data(); - self.set_idx(i, ver.clone())?; - return Ok(None); - } else { - let vers = self.versions[i + 1..].to_vec(); - self.versions.extend(vers.iter().cloned()); - let (free_version, to_free) = ver.object.as_ref().unwrap().init_free_version(fi); - if to_free { - self.add_version_filemata(free_version)?; - } + if version.header.version_type == VersionType::Object && version.header.version_id == fi.version_id { + found_index = Some(i); + break; } + } + let Some(i) = found_index else { if fi.deleted { self.add_version_filemata(ventry)?; - } - if self.shared_data_dir_count(ver.object.as_ref().unwrap().version_id, ver.object.as_ref().unwrap().data_dir) > 0 { return Ok(None); } - return Ok(ver.object.as_ref().unwrap().data_dir); + return Err(Error::FileVersionNotFound); + }; + + let mut ver = self.get_idx(i)?; + + let Some(obj) = &mut ver.object else { + if fi.deleted { + self.add_version_filemata(ventry)?; + return Ok(None); + } + return Err(Error::FileVersionNotFound); + }; + + let obj_version_id = obj.version_id; + let obj_data_dir = obj.data_dir; + + if fi.expire_restored { + obj.remove_restore_hdrs(); + self.set_idx(i, ver)?; + } else if fi.transition_status == TRANSITION_COMPLETE { + obj.set_transition(fi); + obj.reset_inline_data(); + self.set_idx(i, ver)?; + } else { + self.versions.remove(i); + + let (free_version, to_free) = obj.init_free_version(fi); + + if to_free { + self.add_version_filemata(free_version)?; + } } if fi.deleted { self.add_version_filemata(ventry)?; + } + + if self.shared_data_dir_count(obj_version_id, obj_data_dir) > 0 { return Ok(None); } - Err(Error::FileVersionNotFound) + Ok(obj_data_dir) } pub fn into_fileinfo( diff --git a/rustfs/src/admin/handlers/event.rs b/rustfs/src/admin/handlers/event.rs index f52df1fd..513f8696 100644 --- a/rustfs/src/admin/handlers/event.rs +++ b/rustfs/src/admin/handlers/event.rs @@ -227,7 +227,7 @@ impl Operation for NotificationTarget { let port = url .port_or_known_default() .ok_or_else(|| s3_error!(InvalidArgument, "endpoint missing port"))?; - let addr = format!("{}:{}", host, port); + let addr = format!("{host}:{port}"); // First, try to parse as SocketAddr (IP:port) if addr.parse::().is_err() { // If not an IP:port, try DNS resolution diff --git a/rustfs/src/server/http.rs b/rustfs/src/server/http.rs index 1ff4e67a..71bc37e0 100644 --- a/rustfs/src/server/http.rs +++ b/rustfs/src/server/http.rs @@ -144,9 +144,9 @@ pub async fn start_http_server( for domain in &opt.server_domains { domain_sets.insert(domain.to_string()); if let Some((host, _)) = domain.split_once(':') { - domain_sets.insert(format!("{}:{}", host, server_port)); + domain_sets.insert(format!("{host}:{server_port}")); } else { - domain_sets.insert(format!("{}:{}", domain, server_port)); + domain_sets.insert(format!("{domain}:{server_port}")); } }