From 3af96e506b2bc0178280eb7b5bffc6d5ddfc84ee Mon Sep 17 00:00:00 2001 From: Yuang Gao Date: Sat, 23 May 2026 14:42:11 -0700 Subject: [PATCH 1/3] feat(delete): add conditional delete with if-match --- .github/services/s3/0_minio_s3/action.yml | 2 +- .../s3/minio_s3_with_anonymous/action.yml | 2 +- .../minio_s3_with_list_objects_v1/action.yml | 2 +- .../s3/minio_s3_with_versioning/action.yml | 2 +- bindings/nodejs/generated.d.ts | 5 ++ bindings/nodejs/src/options.rs | 7 +++ bindings/python/python/opendal/operator.pyi | 2 + bindings/python/src/operator.rs | 16 +++++-- bindings/python/src/options.rs | 3 ++ core/core/src/layers/correctness_check.rs | 8 ++++ core/core/src/raw/ops.rs | 16 +++++++ core/core/src/types/capability.rs | 2 + .../src/types/operator/operator_futures.rs | 6 +++ core/core/src/types/options.rs | 12 +++++ core/services/s3/src/backend.rs | 1 + core/services/s3/src/core.rs | 5 ++ core/services/s3/src/deleter.rs | 17 +++++++ core/tests/behavior/async_delete.rs | 48 +++++++++++++++++++ 18 files changed, 148 insertions(+), 8 deletions(-) diff --git a/.github/services/s3/0_minio_s3/action.yml b/.github/services/s3/0_minio_s3/action.yml index d9a4c3a6cf8a..66c619db0723 100644 --- a/.github/services/s3/0_minio_s3/action.yml +++ b/.github/services/s3/0_minio_s3/action.yml @@ -43,5 +43,5 @@ runs: OPENDAL_S3_ACCESS_KEY_ID=minioadmin OPENDAL_S3_SECRET_ACCESS_KEY=minioadmin OPENDAL_S3_REGION=us-east-1 - OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false + OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false,delete_with_if_match=false EOF diff --git a/.github/services/s3/minio_s3_with_anonymous/action.yml b/.github/services/s3/minio_s3_with_anonymous/action.yml index d3dd83d3a6dc..0f18ebd5399f 100644 --- a/.github/services/s3/minio_s3_with_anonymous/action.yml +++ b/.github/services/s3/minio_s3_with_anonymous/action.yml @@ -49,5 +49,5 @@ runs: OPENDAL_S3_REGION=us-east-1 OPENDAL_S3_ALLOW_ANONYMOUS=on OPENDAL_S3_DISABLE_EC2_METADATA=on - OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false + OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false,delete_with_if_match=false EOF diff --git a/.github/services/s3/minio_s3_with_list_objects_v1/action.yml b/.github/services/s3/minio_s3_with_list_objects_v1/action.yml index a350def1f3a7..77ee3850193b 100644 --- a/.github/services/s3/minio_s3_with_list_objects_v1/action.yml +++ b/.github/services/s3/minio_s3_with_list_objects_v1/action.yml @@ -44,5 +44,5 @@ runs: OPENDAL_S3_SECRET_ACCESS_KEY=minioadmin OPENDAL_S3_REGION=us-east-1 OPENDAL_S3_DISABLE_LIST_OBJECTS_V2=true - OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false + OPENDAL_TEST_CAPABILITY_OVERRIDES=stat_with_version=false,read_with_version=false,delete_with_version=false,list_with_versions=false,list_with_deleted=false,write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false,delete_with_if_match=false EOF diff --git a/.github/services/s3/minio_s3_with_versioning/action.yml b/.github/services/s3/minio_s3_with_versioning/action.yml index 361bb057649d..f3997717b1a5 100644 --- a/.github/services/s3/minio_s3_with_versioning/action.yml +++ b/.github/services/s3/minio_s3_with_versioning/action.yml @@ -44,5 +44,5 @@ runs: OPENDAL_S3_ACCESS_KEY_ID=minioadmin OPENDAL_S3_SECRET_ACCESS_KEY=minioadmin OPENDAL_S3_REGION=us-east-1 - OPENDAL_TEST_CAPABILITY_OVERRIDES=write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false + OPENDAL_TEST_CAPABILITY_OVERRIDES=write_can_append=false,copy_with_if_not_exists=false,copy_with_if_match=false,delete_with_if_match=false EOF diff --git a/bindings/nodejs/generated.d.ts b/bindings/nodejs/generated.d.ts index 8ac5619d1bac..cdaeac778bd6 100644 --- a/bindings/nodejs/generated.d.ts +++ b/bindings/nodejs/generated.d.ts @@ -1101,6 +1101,11 @@ export interface DeleteOptions { version?: string /** Whether to delete recursively. */ recursive?: boolean + /** * Sets if-match condition for this operation. + * If file exists and its etag does not match, an error of kind + * `ConditionNotMatch` will be returned. + */ + ifMatch?: string } export declare const enum EntryMode { diff --git a/bindings/nodejs/src/options.rs b/bindings/nodejs/src/options.rs index ccb1cae5b020..43b4d8fefeb3 100644 --- a/bindings/nodejs/src/options.rs +++ b/bindings/nodejs/src/options.rs @@ -512,6 +512,12 @@ pub struct DeleteOptions { pub version: Option, /// Whether to delete recursively. pub recursive: Option, + /** + * Sets if-match condition for this operation. + * If file exists and its etag does not match, an error of kind + * `ConditionNotMatch` will be returned. + */ + pub if_match: Option, } impl From for opendal::options::DeleteOptions { @@ -519,6 +525,7 @@ impl From for opendal::options::DeleteOptions { Self { version: value.version, recursive: value.recursive.unwrap_or_default(), + if_match: value.if_match, } } } diff --git a/bindings/python/python/opendal/operator.pyi b/bindings/python/python/opendal/operator.pyi index 8ecb4dd2cadd..9831a7281ceb 100644 --- a/bindings/python/python/opendal/operator.pyi +++ b/bindings/python/python/opendal/operator.pyi @@ -340,6 +340,7 @@ class AsyncOperator: *, version: builtins.str | None = None, recursive: builtins.bool | None = None, + if_match: builtins.str | None = None, ) -> collections.abc.Awaitable[None]: r""" Delete a file at the given path. @@ -3069,6 +3070,7 @@ class Operator: *, version: builtins.str | None = None, recursive: builtins.bool | None = None, + if_match: builtins.str | None = None, ) -> None: r""" Delete a file at the given path. diff --git a/bindings/python/src/operator.rs b/bindings/python/src/operator.rs index e21fbbc0a4a2..4ad357a68527 100644 --- a/bindings/python/src/operator.rs +++ b/bindings/python/src/operator.rs @@ -527,18 +527,22 @@ impl Operator { /// The version of the file to delete. Only supported on version-aware backends. /// recursive : bool, optional /// If True, delete the path recursively. Only supported on backends that support recursive delete. - #[pyo3(signature = (path, *, version=None, recursive=None))] + /// if_match : str, optional + /// If set, the delete will only succeed when the existing object's ETag matches. + #[pyo3(signature = (path, *, version=None, recursive=None, if_match=None))] pub fn delete( &self, path: PathBuf, version: Option, recursive: Option, + if_match: Option, ) -> PyResult<()> { let path = path.to_string_lossy().to_string(); - if version.is_some() || recursive.is_some() { + if version.is_some() || recursive.is_some() || if_match.is_some() { let opts = ocore::options::DeleteOptions { version, recursive: recursive.unwrap_or(false), + if_match, }; self.core.delete_options(&path, opts).map_err(format_pyerr) } else { @@ -1324,21 +1328,25 @@ impl AsyncOperator { /// The version of the file to delete. Only supported on version-aware backends. /// recursive : bool, optional /// If True, delete the path recursively. Only supported on backends that support recursive delete. - #[pyo3(signature = (path, *, version=None, recursive=None))] + /// if_match : str, optional + /// If set, the delete will only succeed when the existing object's ETag matches. + #[pyo3(signature = (path, *, version=None, recursive=None, if_match=None))] pub fn delete<'p>( &'p self, py: Python<'p>, path: PathBuf, version: Option, recursive: Option, + if_match: Option, ) -> PyResult> { let this = self.core.clone(); let path = path.to_string_lossy().to_string(); future_into_py(py, async move { - if version.is_some() || recursive.is_some() { + if version.is_some() || recursive.is_some() || if_match.is_some() { let opts = ocore::options::DeleteOptions { version, recursive: recursive.unwrap_or(false), + if_match, }; this.delete_options(&path, opts).await.map_err(format_pyerr) } else { diff --git a/bindings/python/src/options.rs b/bindings/python/src/options.rs index 1d37e0e7112e..3909c1d8d916 100644 --- a/bindings/python/src/options.rs +++ b/bindings/python/src/options.rs @@ -289,6 +289,7 @@ impl From for ocore::options::StatOptions { pub struct DeleteOptions { pub version: Option, pub recursive: Option, + pub if_match: Option, } impl<'a, 'py> FromPyObject<'a, 'py> for DeleteOptions { @@ -300,6 +301,7 @@ impl<'a, 'py> FromPyObject<'a, 'py> for DeleteOptions { Ok(Self { version: extract_optional(&dict, "version")?, recursive: extract_optional(&dict, "recursive")?, + if_match: extract_optional(&dict, "if_match")?, }) } } @@ -309,6 +311,7 @@ impl From for ocore::options::DeleteOptions { Self { version: opts.version, recursive: opts.recursive.unwrap_or(false), + if_match: opts.if_match, } } } diff --git a/core/core/src/layers/correctness_check.rs b/core/core/src/layers/correctness_check.rs index 020acf4075c5..f5bc911c5997 100644 --- a/core/core/src/layers/correctness_check.rs +++ b/core/core/src/layers/correctness_check.rs @@ -276,6 +276,14 @@ impl CheckWrapper { )); } + if args.if_match().is_some() && !self.info.full_capability().delete_with_if_match { + return Err(new_unsupported_error( + &self.info, + Operation::Delete, + "if_match", + )); + } + Ok(()) } } diff --git a/core/core/src/raw/ops.rs b/core/core/src/raw/ops.rs index 30ca7395c3a5..bf5d9326b7e8 100644 --- a/core/core/src/raw/ops.rs +++ b/core/core/src/raw/ops.rs @@ -44,6 +44,7 @@ impl OpCreateDir { pub struct OpDelete { version: Option, recursive: bool, + if_match: Option, } impl OpDelete { @@ -66,6 +67,15 @@ impl OpDelete { self } + /// Set the if_match condition for this delete operation. + /// + /// When set, the delete will only proceed if the existing object's ETag + /// matches the given value. + pub fn with_if_match(mut self, if_match: impl Into) -> Self { + self.if_match = Some(if_match.into()); + self + } + /// Get the version of this delete operation. pub fn version(&self) -> Option<&str> { self.version.as_deref() @@ -75,6 +85,11 @@ impl OpDelete { pub fn recursive(&self) -> bool { self.recursive } + + /// Get the if_match condition. + pub fn if_match(&self) -> Option<&str> { + self.if_match.as_deref() + } } impl From for OpDelete { @@ -82,6 +97,7 @@ impl From for OpDelete { Self { version: value.version, recursive: value.recursive, + if_match: value.if_match, } } } diff --git a/core/core/src/types/capability.rs b/core/core/src/types/capability.rs index 032077765a3c..106cd28447da 100644 --- a/core/core/src/types/capability.rs +++ b/core/core/src/types/capability.rs @@ -149,6 +149,8 @@ pub struct Capability { pub delete_with_version: bool, /// Indicates if recursive delete operations are supported. pub delete_with_recursive: bool, + /// Indicates if conditional delete operations using If-Match are supported. + pub delete_with_if_match: bool, /// Maximum size supported for single delete operations. pub delete_max_size: Option, diff --git a/core/core/src/types/operator/operator_futures.rs b/core/core/src/types/operator/operator_futures.rs index 34e22711767c..362b082d517d 100644 --- a/core/core/src/types/operator/operator_futures.rs +++ b/core/core/src/types/operator/operator_futures.rs @@ -1279,6 +1279,12 @@ impl>> FutureDelete { self.args.recursive = recursive; self } + + /// Set `if_match` for this delete operation. + pub fn if_match(mut self, etag: &str) -> Self { + self.args.if_match = Some(etag.to_string()); + self + } } /// Future that generated by [`Operator::deleter_with`]. diff --git a/core/core/src/types/options.rs b/core/core/src/types/options.rs index b5bdc9102e52..78667034a721 100644 --- a/core/core/src/types/options.rs +++ b/core/core/src/types/options.rs @@ -31,6 +31,18 @@ pub struct DeleteOptions { /// - If `true`, all entries under the path (or sharing the prefix for file-like paths) /// will be removed. pub recursive: bool, + /// Sets the condition that delete will succeed only if the existing + /// object has the given ETag. + /// + /// ### Capability + /// + /// Check [`Capability::delete_with_if_match`] before using this feature. + /// + /// ### Behavior + /// + /// - If supported, the delete will only succeed when the existing object's + /// ETag matches the given value. + pub if_match: Option, } /// Options for list operations. diff --git a/core/services/s3/src/backend.rs b/core/services/s3/src/backend.rs index e00b70f9a6a8..94fac062bc83 100644 --- a/core/services/s3/src/backend.rs +++ b/core/services/s3/src/backend.rs @@ -970,6 +970,7 @@ impl Builder for S3Builder { delete: true, delete_max_size: Some(DEFAULT_BATCH_MAX_OPERATIONS), delete_with_version: true, + delete_with_if_match: true, copy: true, copy_can_multi: true, diff --git a/core/services/s3/src/core.rs b/core/services/s3/src/core.rs index 944a1d32c24e..481c6815f53b 100644 --- a/core/services/s3/src/core.rs +++ b/core/services/s3/src/core.rs @@ -635,6 +635,11 @@ impl S3Core { let mut req = Request::delete(&url); + // Set conditional delete header. + if let Some(if_match) = args.if_match() { + req = req.header(IF_MATCH, if_match); + } + // Set request payer header if enabled. req = self.insert_request_payer_header(req); diff --git a/core/services/s3/src/deleter.rs b/core/services/s3/src/deleter.rs index 48831ffbbac3..882e9bdeed54 100644 --- a/core/services/s3/src/deleter.rs +++ b/core/services/s3/src/deleter.rs @@ -59,6 +59,23 @@ impl oio::BatchDelete for S3Deleter { } async fn delete_batch(&self, batch: Vec<(String, OpDelete)>) -> Result { + // The S3 DeleteObjects (batch) API does not allow per-object If-Match + // headers, so we fall back to per-object DeleteObject calls whenever + // any entry in the batch carries an `if_match` condition. + if batch.iter().any(|(_, op)| op.if_match().is_some()) { + let mut batched_result = BatchDeleteResult { + succeeded: Vec::with_capacity(batch.len()), + failed: Vec::new(), + }; + for (path, op) in batch { + match self.delete_once(path.clone(), op.clone()).await { + Ok(()) => batched_result.succeeded.push((path, op)), + Err(err) => batched_result.failed.push((path, op, err)), + } + } + return Ok(batched_result); + } + let resp = self.core.s3_delete_objects(&batch).await?; let status = resp.status(); diff --git a/core/tests/behavior/async_delete.rs b/core/tests/behavior/async_delete.rs index 3d1893a7cd46..83800f8642f2 100644 --- a/core/tests/behavior/async_delete.rs +++ b/core/tests/behavior/async_delete.rs @@ -48,6 +48,13 @@ pub fn tests(op: &Operator, tests: &mut Vec) { tests.extend(async_trials!(op, test_remove_all_with_prefix_exists)); } } + if cap.delete_with_if_match { + tests.extend(async_trials!( + op, + test_delete_with_if_match_match, + test_delete_with_if_match_mismatch + )); + } } } @@ -414,3 +421,44 @@ pub async fn test_batch_delete_with_version(op: Operator) -> Result<()> { Ok(()) } + +/// Delete with a matching `If-Match` ETag should succeed and remove the object. +pub async fn test_delete_with_if_match_match(op: Operator) -> Result<()> { + if !op.info().full_capability().delete_with_if_match { + return Ok(()); + } + + let (path, content, _) = TEST_FIXTURE.new_file(op.clone()); + op.write(&path, content).await.expect("write must succeed"); + + let meta = op.stat(&path).await.expect("stat must succeed"); + let etag = meta.etag().expect("etag must be present"); + + op.delete_with(&path).if_match(etag).await?; + + assert!(!op.exists(&path).await?); + + Ok(()) +} + +/// Delete with a non-matching `If-Match` ETag should fail with +/// [`ErrorKind::ConditionNotMatch`] and leave the object intact. +pub async fn test_delete_with_if_match_mismatch(op: Operator) -> Result<()> { + if !op.info().full_capability().delete_with_if_match { + return Ok(()); + } + + let (path, content, _) = TEST_FIXTURE.new_file(op.clone()); + op.write(&path, content).await.expect("write must succeed"); + + let err = op + .delete_with(&path) + .if_match("\"this-etag-does-not-match\"") + .await + .expect_err("delete must fail when etag mismatches"); + assert_eq!(err.kind(), ErrorKind::ConditionNotMatch); + + assert!(op.exists(&path).await?); + + Ok(()) +} From 7e5d695184865dcb6b2f49f3863c73e4ebc7c008 Mon Sep 17 00:00:00 2001 From: Yuang Gao Date: Sat, 23 May 2026 14:46:44 -0700 Subject: [PATCH 2/3] ci: retrigger From 5db69411851061dc0fcb198b533e76e23a54ece3 Mon Sep 17 00:00:00 2001 From: Yuang Gao Date: Sat, 23 May 2026 23:42:22 -0700 Subject: [PATCH 3/3] support if-match in batch DeleteObjects --- core/services/s3/src/core.rs | 14 ++++++++++++++ core/services/s3/src/deleter.rs | 17 ----------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/core/services/s3/src/core.rs b/core/services/s3/src/core.rs index 481c6815f53b..1fc7bc929b12 100644 --- a/core/services/s3/src/core.rs +++ b/core/services/s3/src/core.rs @@ -1211,6 +1211,7 @@ impl S3Core { .map(|(path, op)| DeleteObjectsRequestObject { key: build_abs_path(&self.root, path), version_id: op.version().map(|v| v.to_owned()), + etag: op.if_match().map(|v| v.to_owned()), }) .collect(), }) @@ -1385,6 +1386,8 @@ pub struct DeleteObjectsRequestObject { pub key: String, #[serde(skip_serializing_if = "Option::is_none")] pub version_id: Option, + #[serde(rename = "ETag", skip_serializing_if = "Option::is_none")] + pub etag: Option, } /// Result of DeleteObjects. @@ -1647,10 +1650,17 @@ mod tests { DeleteObjectsRequestObject { key: "sample1.txt".to_string(), version_id: None, + etag: None, }, DeleteObjectsRequestObject { key: "sample2.txt".to_string(), version_id: Some("11111".to_owned()), + etag: None, + }, + DeleteObjectsRequestObject { + key: "sample3.txt".to_string(), + version_id: None, + etag: Some("\"d41d8cd98f00b204e9800998ecf8427e\"".to_owned()), }, ], }; @@ -1668,6 +1678,10 @@ mod tests { sample2.txt 11111 + + sample3.txt + "d41d8cd98f00b204e9800998ecf8427e" + "# // Cleanup space and new line .replace([' ', '\n'], "") diff --git a/core/services/s3/src/deleter.rs b/core/services/s3/src/deleter.rs index 882e9bdeed54..48831ffbbac3 100644 --- a/core/services/s3/src/deleter.rs +++ b/core/services/s3/src/deleter.rs @@ -59,23 +59,6 @@ impl oio::BatchDelete for S3Deleter { } async fn delete_batch(&self, batch: Vec<(String, OpDelete)>) -> Result { - // The S3 DeleteObjects (batch) API does not allow per-object If-Match - // headers, so we fall back to per-object DeleteObject calls whenever - // any entry in the batch carries an `if_match` condition. - if batch.iter().any(|(_, op)| op.if_match().is_some()) { - let mut batched_result = BatchDeleteResult { - succeeded: Vec::with_capacity(batch.len()), - failed: Vec::new(), - }; - for (path, op) in batch { - match self.delete_once(path.clone(), op.clone()).await { - Ok(()) => batched_result.succeeded.push((path, op)), - Err(err) => batched_result.failed.push((path, op, err)), - } - } - return Ok(batched_result); - } - let resp = self.core.s3_delete_objects(&batch).await?; let status = resp.status();