Skip to content

Commit 6093331

Browse files
committed
feat(s3): add conditional headers (If-Match, If-None-Match) for copy/delete
Signed-off-by: Shashank Singh <shashanksgh3@gmail.com>
1 parent e2b1794 commit 6093331

File tree

8 files changed

+256
-6
lines changed

8 files changed

+256
-6
lines changed

core/core/src/raw/ops.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ impl OpCreateDir {
4444
pub struct OpDelete {
4545
version: Option<String>,
4646
recursive: bool,
47+
if_match: Option<String>,
4748
}
4849

4950
impl OpDelete {
@@ -75,13 +76,25 @@ impl OpDelete {
7576
pub fn recursive(&self) -> bool {
7677
self.recursive
7778
}
79+
80+
/// Set the If-Match of the option
81+
pub fn with_if_match(mut self, if_match: &str) -> Self {
82+
self.if_match = Some(if_match.to_string());
83+
self
84+
}
85+
86+
/// Get If-Match from option
87+
pub fn if_match(&self) -> Option<&str> {
88+
self.if_match.as_deref()
89+
}
7890
}
7991

8092
impl From<options::DeleteOptions> for OpDelete {
8193
fn from(value: options::DeleteOptions) -> Self {
8294
Self {
8395
version: value.version,
8496
recursive: value.recursive,
97+
if_match: value.if_match,
8598
}
8699
}
87100
}
@@ -878,6 +891,7 @@ impl From<options::WriteOptions> for (OpWrite, OpWriter) {
878891
#[derive(Debug, Clone, Default)]
879892
pub struct OpCopy {
880893
if_not_exists: bool,
894+
if_match: Option<String>,
881895
}
882896

883897
impl OpCopy {
@@ -899,6 +913,17 @@ impl OpCopy {
899913
pub fn if_not_exists(&self) -> bool {
900914
self.if_not_exists
901915
}
916+
917+
/// Set the If-Match of the option
918+
pub fn with_if_match(mut self, if_match: &str) -> Self {
919+
self.if_match = Some(if_match.to_string());
920+
self
921+
}
922+
923+
/// Get If-Match from option
924+
pub fn if_match(&self) -> Option<&str> {
925+
self.if_match.as_deref()
926+
}
902927
}
903928

904929
/// Args for `rename` operation.

core/core/src/types/capability.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,13 +145,17 @@ pub struct Capability {
145145
pub delete_with_version: bool,
146146
/// Indicates if recursive delete operations are supported.
147147
pub delete_with_recursive: bool,
148+
/// Indicates if conditional delete operations with if-match are supported.
149+
pub delete_with_if_match: bool,
148150
/// Maximum size supported for single delete operations.
149151
pub delete_max_size: Option<usize>,
150152

151153
/// Indicates if copy operations are supported.
152154
pub copy: bool,
153155
/// Indicates if conditional copy operations with if-not-exists are supported.
154156
pub copy_with_if_not_exists: bool,
157+
/// Indicates if conditional copy operations with if-match are supported.
158+
pub copy_with_if_match: bool,
155159

156160
/// Indicates if rename operations are supported.
157161
pub rename: bool,

core/core/src/types/operator/operator_futures.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,35 @@ impl<F: Future<Output = Result<()>>> FutureDelete<F> {
12631263
self.args.recursive = recursive;
12641264
self
12651265
}
1266+
1267+
/// Sets If-Match header for this delete request.
1268+
///
1269+
/// ### Behavior
1270+
///
1271+
/// - If supported, the delete operation will only succeed if the file's ETag matches the specified value
1272+
/// - The value should be a valid ETag string
1273+
/// - If not supported, the value will be ignored
1274+
///
1275+
/// This operation provides conditional delete functionality based on ETag matching.
1276+
///
1277+
/// ### Example
1278+
///
1279+
/// ```
1280+
/// # use opendal_core::Result;
1281+
/// # use opendal_core::Operator;
1282+
///
1283+
/// # async fn test(op: Operator) -> Result<()> {
1284+
/// let _ = op
1285+
/// .delete_with("path/to/file")
1286+
/// .if_match("\"686897696a7c876b7e\"")
1287+
/// .await?;
1288+
/// # Ok(())
1289+
/// # }
1290+
/// ```
1291+
pub fn if_match(mut self, s: &str) -> Self {
1292+
self.args.if_match = Some(s.to_string());
1293+
self
1294+
}
12661295
}
12671296

12681297
/// Future that generated by [`Operator::deleter_with`].
@@ -1420,4 +1449,33 @@ impl<F: Future<Output = Result<()>>> FutureCopy<F> {
14201449
self.args.0.if_not_exists = v;
14211450
self
14221451
}
1452+
1453+
/// Sets If-Match header for this copy request.
1454+
///
1455+
/// ### Behavior
1456+
///
1457+
/// - If supported, the copy operation will only succeed if the source's ETag matches the specified value
1458+
/// - The value should be a valid ETag string
1459+
/// - If not supported, the value will be ignored
1460+
///
1461+
/// This operation provides conditional copy functionality based on ETag matching.
1462+
///
1463+
/// ### Example
1464+
///
1465+
/// ```
1466+
/// # use opendal_core::Result;
1467+
/// # use opendal_core::Operator;
1468+
///
1469+
/// # async fn test(op: Operator) -> Result<()> {
1470+
/// let _ = op
1471+
/// .copy_with("source/path", "target/path")
1472+
/// .if_match("\"686897696a7c876b7e\"")
1473+
/// .await?;
1474+
/// # Ok(())
1475+
/// # }
1476+
/// ```
1477+
pub fn if_match(mut self, s: &str) -> Self {
1478+
self.args.0.if_match = Some(s.to_string());
1479+
self
1480+
}
14231481
}

core/core/src/types/options.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,17 @@ use crate::raw::{BytesRange, Timestamp};
2121
use std::collections::HashMap;
2222

2323
/// Options for delete operations.
24-
#[derive(Debug, Clone, Default, Eq, PartialEq)]
24+
#[derive(Debug, Clone, Default)]
2525
pub struct DeleteOptions {
2626
/// The version of the file to delete.
27-
pub version: Option<String>,
27+
pub(crate) version: Option<String>,
28+
/// Set `if_match` for this operation.
29+
///
30+
/// This option can be used to check if the file's `ETag` matches the given `ETag`.
31+
///
32+
/// If file exists and it's etag doesn't match, an error with kind [`ErrorKind::ConditionNotMatch`]
33+
/// will be returned.
34+
pub(crate) if_match: Option<String>,
2835
/// Whether to delete the target recursively.
2936
///
3037
/// - If `false`, behaves like the traditional single-object delete.
@@ -532,4 +539,12 @@ pub struct CopyOptions {
532539
/// This operation provides a way to ensure copy operations only create new resources
533540
/// without overwriting existing ones, useful for implementing "copy if not exists" logic.
534541
pub if_not_exists: bool,
542+
543+
/// Set `if_match` for this operation.
544+
///
545+
/// This option can be used to check if the source file's `ETag` matches the given `ETag`.
546+
///
547+
/// If file exists and it's etag doesn't match, an error with kind [`ErrorKind::ConditionNotMatch`]
548+
/// will be returned.
549+
pub if_match: Option<String>,
535550
}

core/services/s3/src/backend.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -922,12 +922,15 @@ impl Builder for S3Builder {
922922
},
923923

924924
delete: true,
925+
delete_with_if_match: true,
925926
delete_max_size: Some(delete_max_size),
926927
delete_with_version: config.enable_versioning,
927928

928929
copy: true,
930+
copy_with_if_not_exists: true,
931+
copy_with_if_match: true,
929932

930-
list: true,
933+
rename: true,
931934
list_with_limit: true,
932935
list_with_start_after: true,
933936
list_with_recursive: true,
@@ -1072,8 +1075,8 @@ impl Access for S3Backend {
10721075
Ok((RpList::default(), l))
10731076
}
10741077

1075-
async fn copy(&self, from: &str, to: &str, _args: OpCopy) -> Result<RpCopy> {
1076-
let resp = self.core.s3_copy_object(from, to).await?;
1078+
async fn copy(&self, from: &str, to: &str, args: OpCopy) -> Result<RpCopy> {
1079+
let resp = self.core.s3_copy_object(from, to, args).await?;
10771080

10781081
let status = resp.status();
10791082

core/services/s3/src/core.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,11 @@ impl S3Core {
613613

614614
let mut req = Request::delete(&url);
615615

616+
// Add if_match condition using If-Match header
617+
if let Some(if_match) = args.if_match() {
618+
req = req.header(IF_MATCH, if_match);
619+
}
620+
616621
// Set request payer header if enabled.
617622
req = self.insert_request_payer_header(req);
618623

@@ -625,7 +630,7 @@ impl S3Core {
625630
self.send(req).await
626631
}
627632

628-
pub async fn s3_copy_object(&self, from: &str, to: &str) -> Result<Response<Buffer>> {
633+
pub async fn s3_copy_object(&self, from: &str, to: &str, args: OpCopy) -> Result<Response<Buffer>> {
629634
let from = build_abs_path(&self.root, from);
630635
let to = build_abs_path(&self.root, to);
631636

@@ -673,6 +678,16 @@ impl S3Core {
673678
)
674679
}
675680

681+
// Add if_not_exists condition using If-None-Match header
682+
if args.if_not_exists() {
683+
req = req.header(IF_NONE_MATCH, "*");
684+
}
685+
686+
// Add if_match condition using If-Match header
687+
if let Some(if_match) = args.if_match() {
688+
req = req.header(IF_MATCH, if_match);
689+
}
690+
676691
// Set request payer header if enabled.
677692
req = self.insert_request_payer_header(req);
678693

core/tests/behavior/async_copy.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ pub fn tests(op: &Operator, tests: &mut Vec<Trial>) {
4545
test_copy_with_if_not_exists_to_existing_file
4646
))
4747
}
48+
49+
if cap.read && cap.write && cap.copy && cap.copy_with_if_match {
50+
tests.extend(async_trials!(
51+
op,
52+
test_copy_with_if_match_success,
53+
test_copy_with_if_match_failure
54+
))
55+
}
4856
}
4957

5058
/// Copy a file with ascii name and test contents.
@@ -313,3 +321,68 @@ pub async fn test_copy_with_if_not_exists_to_existing_file(op: Operator) -> Resu
313321
op.delete(&target_path).await.expect("delete must succeed");
314322
Ok(())
315323
}
324+
325+
/// Copy with if_match should succeed when ETag matches.
326+
pub async fn test_copy_with_if_match_success(op: Operator) -> Result<()> {
327+
if !op.info().full_capability().copy_with_if_match {
328+
return Ok(());
329+
}
330+
331+
let source_path = uuid::Uuid::new_v4().to_string();
332+
let (source_content, _) = gen_bytes(op.info().full_capability());
333+
334+
op.write(&source_path, source_content.clone()).await?;
335+
336+
// Get the ETag of the source file
337+
let source_meta = op.stat(&source_path).await?;
338+
let etag = source_meta.etag().expect("source should have etag");
339+
340+
let target_path = uuid::Uuid::new_v4().to_string();
341+
342+
// Copy with matching ETag should succeed
343+
op.copy_with(&source_path, &target_path)
344+
.if_match(etag)
345+
.await?;
346+
347+
let target_content = op
348+
.read(&target_path)
349+
.await
350+
.expect("read must succeed")
351+
.to_bytes();
352+
assert_eq!(
353+
format!("{:x}", Sha256::digest(target_content)),
354+
format!("{:x}", Sha256::digest(&source_content)),
355+
);
356+
357+
op.delete(&source_path).await.expect("delete must succeed");
358+
op.delete(&target_path).await.expect("delete must succeed");
359+
Ok(())
360+
}
361+
362+
/// Copy with if_match should fail when ETag doesn't match.
363+
pub async fn test_copy_with_if_match_failure(op: Operator) -> Result<()> {
364+
if !op.info().full_capability().copy_with_if_match {
365+
return Ok(());
366+
}
367+
368+
let source_path = uuid::Uuid::new_v4().to_string();
369+
let (source_content, _) = gen_bytes(op.info().full_capability());
370+
371+
op.write(&source_path, source_content.clone()).await?;
372+
373+
let target_path = uuid::Uuid::new_v4().to_string();
374+
375+
// Copy with non-matching ETag should fail
376+
let err = op
377+
.copy_with(&source_path, &target_path)
378+
.if_match("invalid-etag")
379+
.await
380+
.expect_err("copy must fail");
381+
assert_eq!(err.kind(), ErrorKind::ConditionNotMatch);
382+
383+
// Verify target file was not created
384+
assert!(!op.exists(&target_path).await?);
385+
386+
op.delete(&source_path).await.expect("delete must succeed");
387+
Ok(())
388+
}

core/tests/behavior/async_delete.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ pub fn tests(op: &Operator, tests: &mut Vec<Trial>) {
4848
tests.extend(async_trials!(op, test_remove_all_with_prefix_exists));
4949
}
5050
}
51+
if cap.delete_with_if_match {
52+
tests.extend(async_trials!(
53+
op,
54+
test_delete_with_if_match_success,
55+
test_delete_with_if_match_failure
56+
));
57+
}
5158
}
5259
}
5360

@@ -381,3 +388,53 @@ pub async fn test_batch_delete_with_version(op: Operator) -> Result<()> {
381388

382389
Ok(())
383390
}
391+
392+
/// Delete with if_match should succeed when ETag matches.
393+
pub async fn test_delete_with_if_match_success(op: Operator) -> Result<()> {
394+
if !op.info().full_capability().delete_with_if_match {
395+
return Ok(());
396+
}
397+
398+
let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
399+
400+
op.write(&path, content).await.expect("write must succeed");
401+
402+
// Get the ETag of the file
403+
let meta = op.stat(&path).await.expect("stat must succeed");
404+
let etag = meta.etag().expect("file should have etag");
405+
406+
// Delete with matching ETag should succeed
407+
op.delete_with(&path).if_match(etag).await?;
408+
409+
// Verify file is deleted
410+
assert!(!op.exists(&path).await?);
411+
412+
Ok(())
413+
}
414+
415+
/// Delete with if_match should fail when ETag doesn't match.
416+
pub async fn test_delete_with_if_match_failure(op: Operator) -> Result<()> {
417+
if !op.info().full_capability().delete_with_if_match {
418+
return Ok(());
419+
}
420+
421+
let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
422+
423+
op.write(&path, content).await.expect("write must succeed");
424+
425+
// Delete with non-matching ETag should fail
426+
let err = op
427+
.delete_with(&path)
428+
.if_match("invalid-etag")
429+
.await
430+
.expect_err("delete must fail");
431+
assert_eq!(err.kind(), ErrorKind::ConditionNotMatch);
432+
433+
// Verify file still exists
434+
assert!(op.exists(&path).await?);
435+
436+
// Clean up
437+
op.delete(&path).await.expect("delete must succeed");
438+
439+
Ok(())
440+
}

0 commit comments

Comments
 (0)