fix(object): make version undo preconditions atomic (#5225)

* fix(object): make version undo preconditions atomic

* fix(object): fence metadata-only undo copies

* fix(object): order versions by commit time

crates/ecstore/src/object_api/types.rs

@@ -30,6 +30,8 @@ pub struct ObjectOptions {
     pub delete_prefix: bool,
     pub delete_prefix_object: bool,
     pub version_id: Option<String>,
+    /// RustFS-only compare-and-set condition checked under the object write lock.
+    pub expected_current_version_id: Option<String>,
     pub no_lock: bool,
     /// True when an upper layer already holds the object read lock before
     /// forwarding a no_lock read to the set layer.

crates/ecstore/src/set_disk/ops/object.rs

@@ -1001,11 +1001,7 @@ impl SetDisks {
                 let _ = user_defined.remove(AMZ_STORAGE_CLASS);
             }
 
-            let mod_time = if let Some(mod_time) = opts.mod_time {
-                Some(mod_time)
-            } else {
-                Some(OffsetDateTime::now_utc())
-            };
+            let mod_time = opts.mod_time;
 
             // Drop any disk whose shard did not fully commit (offline at writer
             // setup, short write, or a write/shutdown error) so its truncated or
@@ -1075,6 +1071,45 @@ impl SetDisks {
                 object_lock_guard = Some(self.acquire_write_lock_diag("put_object_commit", bucket, object).await?);
             }
 
+            // Generate ordinary PUT timestamps under the commit lock so version
+            // ordering follows durable commit ordering when writers queued on
+            // the same object. Internal callers with an explicit timestamp keep
+            // their supplied value.
+            if opts.mod_time.is_none() {
+                let commit_time = Some(OffsetDateTime::now_utc());
+                for pfi in &mut parts_metadatas {
+                    pfi.mod_time = commit_time;
+                    for part in &mut pfi.parts {
+                        part.mod_time = commit_time;
+                    }
+                }
+            }
+
+            if let Some(expected) = opts.expected_current_version_id.as_deref() {
+                let current = self
+                    .get_object_info(
+                        bucket,
+                        object,
+                        &ObjectOptions {
+                            no_lock: true,
+                            metadata_cache_safe: false,
+                            versioned: true,
+                            ..Default::default()
+                        },
+                    )
+                    .await
+                    .map_err(|err| {
+                        if is_err_object_not_found(&err) || is_err_version_not_found(&err) {
+                            StorageError::PreconditionFailed
+                        } else {
+                            err
+                        }
+                    })?;
+                if current.version_id.map(|version| version.to_string()).as_deref() != Some(expected) {
+                    return Err(StorageError::PreconditionFailed);
+                }
+            }
+
             // Phase 2 (backlog#899): fence the commit on lock loss. If the refresh
             // heartbeat has observed a refresh-quorum loss, another writer may have
             // re-acquired this object's lock; committing now would race a double-write.
@@ -2323,6 +2358,31 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
             )
         };
 
+        if let Some(expected) = dst_opts.expected_current_version_id.as_deref() {
+            let current = self
+                .get_object_info(
+                    dst_bucket,
+                    dst_object,
+                    &ObjectOptions {
+                        no_lock: true,
+                        metadata_cache_safe: false,
+                        versioned: true,
+                        ..Default::default()
+                    },
+                )
+                .await
+                .map_err(|err| {
+                    if is_err_object_not_found(&err) || is_err_version_not_found(&err) {
+                        StorageError::PreconditionFailed
+                    } else {
+                        err
+                    }
+                })?;
+            if current.version_id.map(|version| version.to_string()).as_deref() != Some(expected) {
+                return Err(StorageError::PreconditionFailed);
+            }
+        }
+
         self.invalidate_get_object_metadata_cache(dst_bucket, dst_object).await;
 
         if dst_opts.http_preconditions.is_some()
@@ -2967,6 +3027,34 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks {
             return Ok(ObjectInfo::default());
         }
 
+        if let Some(expected) = opts.expected_current_version_id.as_deref() {
+            let current = self
+                .get_object_info(
+                    bucket,
+                    object,
+                    &ObjectOptions {
+                        no_lock: true,
+                        metadata_cache_safe: false,
+                        versioned: true,
+                        ..Default::default()
+                    },
+                )
+                .await
+                .map_err(|err| {
+                    if is_err_object_not_found(&err) || is_err_version_not_found(&err) {
+                        StorageError::PreconditionFailed
+                    } else {
+                        err
+                    }
+                })?;
+            if !current.delete_marker
+                || current.version_id.map(|version| version.to_string()).as_deref() != Some(expected)
+                || opts.version_id.as_deref() != Some(expected)
+            {
+                return Err(StorageError::PreconditionFailed);
+            }
+        }
+
         // TODO: Lifecycle
 
         let mut version_found = true;

crates/ecstore/src/store/object.rs

@@ -933,7 +933,7 @@ impl ECStore {
         let cp_src_dst_same = path_join_buf(&[src_bucket, &src_object]) == path_join_buf(&[dst_bucket, &dst_object]);
 
         let mut dst_opts = dst_opts.clone();
-        let _dst_lock_guard = if cp_src_dst_same {
+        let _dst_lock_guard = if cp_src_dst_same && dst_opts.expected_current_version_id.is_none() {
             self.acquire_object_write_lock_if_needed("copy_object", dst_bucket, &dst_object, &mut dst_opts)
                 .await?
         } else {
@@ -969,6 +969,7 @@ impl ECStore {
                     no_lock: dst_opts.no_lock,
                     mod_time: dst_opts.mod_time,
                     http_preconditions: dst_opts.http_preconditions.clone(),
+                    expected_current_version_id: dst_opts.expected_current_version_id.clone(),
                     ..Default::default()
                 };
                 return if let Some(reader) = src_info.put_object_reader.as_mut() {
@@ -998,6 +999,7 @@ impl ECStore {
                         no_lock: dst_opts.no_lock,
                         mod_time: dst_opts.mod_time,
                         http_preconditions: dst_opts.http_preconditions.clone(),
+                        expected_current_version_id: dst_opts.expected_current_version_id.clone(),
                         ..Default::default()
                     };
                     return self.pools[pool_idx]
@@ -1024,6 +1026,7 @@ impl ECStore {
             no_lock: dst_opts.no_lock,
             mod_time: dst_opts.mod_time,
             http_preconditions: dst_opts.http_preconditions.clone(),
+            expected_current_version_id: dst_opts.expected_current_version_id.clone(),
             ..Default::default()
         };
 
@@ -1087,9 +1090,12 @@ impl ECStore {
             return Ok(ObjectInfo::default());
         }
 
-        let _object_lock_guard = self
-            .acquire_object_write_lock_if_needed("delete_object", bucket, object, &mut opts)
-            .await?;
+        let _object_lock_guard = if opts.expected_current_version_id.is_none() {
+            self.acquire_object_write_lock_if_needed("delete_object", bucket, object, &mut opts)
+                .await?
+        } else {
+            None
+        };
 
         if opts.delete_prefix {
             self.delete_prefix(bucket, object, &opts).await?;