From c084c60f1794138c23031a43c5d5008e8e3a8552 Mon Sep 17 00:00:00 2001 From: suyanhanx Date: Thu, 27 Apr 2023 14:25:20 +0800 Subject: [PATCH 1/3] add test for read_with_if_none_match Signed-off-by: suyanhanx --- core/src/services/gcs/backend.rs | 2 + core/src/services/http/backend.rs | 2 + core/src/services/obs/backend.rs | 2 + core/src/services/oss/backend.rs | 2 + core/tests/behavior/read_only.rs | 71 +++++++++++++++++++++++++++++++ core/tests/behavior/write.rs | 49 ++++++++++++++++++--- 6 files changed, 122 insertions(+), 6 deletions(-) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index a53a20ab3d62..28ba61e1e803 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -381,6 +381,8 @@ impl Accessor for GcsBackend { read: true, read_can_next: true, + read_with_if_match: true, + read_with_if_none_match: true, write: true, write_with_content_type: true, diff --git a/core/src/services/http/backend.rs b/core/src/services/http/backend.rs index 8a4d3fb6ab76..a39b645ca93f 100644 --- a/core/src/services/http/backend.rs +++ b/core/src/services/http/backend.rs @@ -265,6 +265,8 @@ impl Accessor for HttpBackend { read: true, read_can_next: true, + read_with_if_match: true, + read_with_if_none_match: true, ..Default::default() }); diff --git a/core/src/services/obs/backend.rs b/core/src/services/obs/backend.rs index 4eaf9c010119..e67492c3da55 100644 --- a/core/src/services/obs/backend.rs +++ b/core/src/services/obs/backend.rs @@ -312,6 +312,8 @@ impl Accessor for ObsBackend { read: true, read_can_next: true, + read_with_if_match: true, + read_with_if_none_match: true, write: true, write_with_content_type: true, diff --git a/core/src/services/oss/backend.rs b/core/src/services/oss/backend.rs index 1388c25a6148..47bed95339bc 100644 --- a/core/src/services/oss/backend.rs +++ b/core/src/services/oss/backend.rs @@ -430,6 +430,8 @@ impl Accessor for OssBackend { read: true, read_can_next: true, + read_with_if_match: true, + read_with_if_none_match: true, write: true, write_with_cache_control: true, diff --git a/core/tests/behavior/read_only.rs b/core/tests/behavior/read_only.rs index fdaf939e1f28..5fe416603f6d 100644 --- a/core/tests/behavior/read_only.rs +++ b/core/tests/behavior/read_only.rs @@ -17,6 +17,7 @@ use anyhow::Result; use futures::AsyncReadExt; +use opendal::ops::OpRead; use opendal::ops::OpStat; use opendal::EntryMode; use opendal::ErrorKind; @@ -76,6 +77,8 @@ macro_rules! behavior_read_tests { test_reader_tail, test_read_not_exist, test_read_with_dir_path, + test_read_with_if_match, + test_read_with_if_none_match, ); )* }; @@ -304,3 +307,71 @@ pub async fn test_read_with_dir_path(op: Operator) -> Result<()> { Ok(()) } + +/// Read with if_match should match, else get a ConditionNotMatch error. +pub async fn test_read_with_if_match(op: Operator) -> Result<()> { + if !op.info().capability().read_with_if_match { + return Ok(()); + } + + let path = "normal_file"; + + let meta = op.stat(path).await?; + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_match("invalid_etag"); + + let res = op.read_with(path, op_read).await; + assert!(res.is_err()); + assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch); + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_match(meta.etag().expect("etag must exist")); + + let bs = op + .read_with(path, op_read) + .await + .expect("read must succeed"); + assert_eq!(bs.len(), 262144, "read size"); + assert_eq!( + format!("{:x}", Sha256::digest(&bs)), + "e7541d0f50d2d5c79dc41f28ccba8e0cdfbbc8c4b1aa1a0110184ef0ef67689f", + "read content" + ); + + Ok(()) +} + +/// Read with if_none_match should match, else get a ConditionNotMatch error. +pub async fn test_read_with_if_none_match(op: Operator) -> Result<()> { + if !op.info().capability().read_with_if_none_match { + return Ok(()); + } + + let path = "normal_file"; + + let meta = op.stat(path).await?; + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_none_match(meta.etag().expect("etag must exist")); + + let res = op.read_with(path, op_read).await; + assert!(res.is_err()); + assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch); + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_none_match("invalid_etag"); + + let bs = op + .read_with(path, op_read) + .await + .expect("read must succeed"); + assert_eq!(bs.len(), 262144, "read size"); + assert_eq!( + format!("{:x}", Sha256::digest(&bs)), + "e7541d0f50d2d5c79dc41f28ccba8e0cdfbbc8c4b1aa1a0110184ef0ef67689f", + "read content" + ); + + Ok(()) +} diff --git a/core/tests/behavior/write.rs b/core/tests/behavior/write.rs index c1212cfbb2f6..d15b8b254b8e 100644 --- a/core/tests/behavior/write.rs +++ b/core/tests/behavior/write.rs @@ -95,6 +95,7 @@ macro_rules! behavior_write_tests { test_reader_tail, test_read_not_exist, test_read_with_if_match, + test_read_with_if_none_match, test_fuzz_range_reader, test_fuzz_offset_reader, test_fuzz_part_reader, @@ -585,18 +586,54 @@ pub async fn test_read_with_if_match(op: Operator) -> Result<()> { let meta = op.stat(&path).await?; - let mut op_if_match = OpRead::default(); - op_if_match = op_if_match.with_if_match("invalid_etag"); + let mut op_read = OpRead::default(); + op_read = op_read.with_if_match("invalid_etag"); - let res = op.read_with(&path, op_if_match).await; + let res = op.read_with(&path, op_read).await; assert!(res.is_err()); assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch); - let mut op_if_match = OpRead::default(); - op_if_match = op_if_match.with_if_match(meta.etag().expect("etag must exist")); + let mut op_read = OpRead::default(); + op_read = op_read.with_if_match(meta.etag().expect("etag must exist")); + + let bs = op + .read_with(&path, op_read) + .await + .expect("read must succeed"); + assert_eq!(bs, content); + + op.delete(&path).await.expect("delete must succeed"); + Ok(()) +} + +/// Read with if_none_match should match, else get a ConditionNotMatch error. +pub async fn test_read_with_if_none_match(op: Operator) -> Result<()> { + if !op.info().capability().read_with_if_none_match { + return Ok(()); + } + + let path = uuid::Uuid::new_v4().to_string(); + debug!("Generate a random file: {}", &path); + let (content, _) = gen_bytes(); + + op.write(&path, content.clone()) + .await + .expect("write must succeed"); + + let meta = op.stat(&path).await?; + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_none_match(meta.etag().expect("etag must exist")); + + let res = op.read_with(&path, op_read).await; + assert!(res.is_err()); + assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch); + + let mut op_read = OpRead::default(); + op_read = op_read.with_if_none_match("invalid_etag"); let bs = op - .read_with(&path, op_if_match) + .read_with(&path, op_read) .await .expect("read must succeed"); assert_eq!(bs, content); From addb51870d8ddb1740017b9ecee66920a24904e0 Mon Sep 17 00:00:00 2001 From: suyanhanx Date: Thu, 27 Apr 2023 14:59:50 +0800 Subject: [PATCH 2/3] enable read for webdav Signed-off-by: suyanhanx --- core/src/services/webdav/backend.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/core/src/services/webdav/backend.rs b/core/src/services/webdav/backend.rs index 4a073d6e9fea..383473e3139d 100644 --- a/core/src/services/webdav/backend.rs +++ b/core/src/services/webdav/backend.rs @@ -267,12 +267,18 @@ impl Accessor for WebdavBackend { ma.set_scheme(Scheme::Webdav) .set_root(&self.root) .set_capability(Capability { + stat: true, + read: true, read_can_next: true, + read_with_if_match: true, + read_with_if_none_match: true, + write: true, list: true, copy: true, rename: true, + ..Default::default() }); @@ -287,7 +293,9 @@ impl Accessor for WebdavBackend { } async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead, Self::Reader)> { - let resp = self.webdav_get(path, args.range()).await?; + let resp = self + .webdav_get(path, args.range(), args.if_match(), args.if_none_match()) + .await?; let status = resp.status(); @@ -430,6 +438,8 @@ impl WebdavBackend { &self, path: &str, range: BytesRange, + if_match: Option<&str>, + if_none_match: Option<&str>, ) -> Result> { let p = build_rooted_abs_path(&self.root, path); @@ -445,6 +455,13 @@ impl WebdavBackend { req = req.header(header::RANGE, range.to_header()); } + if let Some(etag) = if_match { + req = req.header(header::IF_MATCH, etag); + } + if let Some(etag) = if_none_match { + req = req.header(header::IF_NONE_MATCH, etag); + } + let req = req .body(AsyncBody::Empty) .map_err(new_request_build_error)?; From 0735e1da406be56a92daa298b2fb8f01b1789879 Mon Sep 17 00:00:00 2001 From: suyanhanx Date: Thu, 27 Apr 2023 15:29:49 +0800 Subject: [PATCH 3/3] Revert "enable read for webdav" This reverts commit addb51870d8ddb1740017b9ecee66920a24904e0. --- core/src/services/webdav/backend.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/core/src/services/webdav/backend.rs b/core/src/services/webdav/backend.rs index 383473e3139d..4a073d6e9fea 100644 --- a/core/src/services/webdav/backend.rs +++ b/core/src/services/webdav/backend.rs @@ -267,18 +267,12 @@ impl Accessor for WebdavBackend { ma.set_scheme(Scheme::Webdav) .set_root(&self.root) .set_capability(Capability { - stat: true, - read: true, read_can_next: true, - read_with_if_match: true, - read_with_if_none_match: true, - write: true, list: true, copy: true, rename: true, - ..Default::default() }); @@ -293,9 +287,7 @@ impl Accessor for WebdavBackend { } async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead, Self::Reader)> { - let resp = self - .webdav_get(path, args.range(), args.if_match(), args.if_none_match()) - .await?; + let resp = self.webdav_get(path, args.range()).await?; let status = resp.status(); @@ -438,8 +430,6 @@ impl WebdavBackend { &self, path: &str, range: BytesRange, - if_match: Option<&str>, - if_none_match: Option<&str>, ) -> Result> { let p = build_rooted_abs_path(&self.root, path); @@ -455,13 +445,6 @@ impl WebdavBackend { req = req.header(header::RANGE, range.to_header()); } - if let Some(etag) = if_match { - req = req.header(header::IF_MATCH, etag); - } - if let Some(etag) = if_none_match { - req = req.header(header::IF_NONE_MATCH, etag); - } - let req = req .body(AsyncBody::Empty) .map_err(new_request_build_error)?;