read/stat non-existent version returns NotFound, delete non-existent …

…version returns Ok

core/src/services/s3/backend.rs

@@ -902,15 +902,15 @@ impl Access for S3Backend {
                 stat_with_override_cache_control: !self.core.disable_stat_with_override,
                 stat_with_override_content_disposition: !self.core.disable_stat_with_override,
                 stat_with_override_content_type: !self.core.disable_stat_with_override,
-                stat_with_versioning: self.core.enable_versioning,
+                stat_with_version: self.core.enable_versioning,
 
                 read: true,
                 read_with_if_match: true,
                 read_with_if_none_match: true,
                 read_with_override_cache_control: true,
                 read_with_override_content_disposition: true,
                 read_with_override_content_type: true,
-                read_with_versioning: self.core.enable_versioning,
+                read_with_version: self.core.enable_versioning,
 
                 write: true,
                 write_can_empty: true,
@@ -933,15 +933,15 @@ impl Access for S3Backend {
                 },
 
                 delete: true,
-                delete_with_versioning: self.core.enable_versioning,
+                delete_with_version: self.core.enable_versioning,
 
                 copy: true,
 
                 list: true,
                 list_with_limit: true,
                 list_with_start_after: true,
                 list_with_recursive: true,
-                list_with_versioning: self.core.enable_versioning,
+                list_with_version: self.core.enable_versioning,
 
                 presign: true,
                 presign_stat: true,

core/src/services/s3/error.rs

@@ -41,7 +41,7 @@ pub(super) fn parse_error(resp: Response<Buffer>) -> Error {
     let (mut kind, mut retryable) = match parts.status.as_u16() {
         403 => (ErrorKind::PermissionDenied, false),
         404 => (ErrorKind::NotFound, false),
-        304 | 400 | 412 => (ErrorKind::ConditionNotMatch, false),
+        304 | 412 => (ErrorKind::ConditionNotMatch, false),
         // Service like R2 could return 499 error with a message like:
         // Client Disconnect, we should retry it.
         499 => (ErrorKind::Unexpected, true),

core/src/types/capability.rs

@@ -68,7 +68,7 @@ pub struct Capability {
     /// if operator supports stat with override content type.
     pub stat_with_override_content_type: bool,
     /// if operator supports stat with versioning.
-    pub stat_with_versioning: bool,
+    pub stat_with_version: bool,
 
     /// If operator supports read.
     pub read: bool,
@@ -83,7 +83,7 @@ pub struct Capability {
     /// if operator supports read with override content type.
     pub read_with_override_content_type: bool,
     /// if operator supports read with versioning.
-    pub read_with_versioning: bool,
+    pub read_with_version: bool,
 
     /// If operator supports write.
     pub write: bool,
@@ -124,7 +124,7 @@ pub struct Capability {
     /// If operator supports delete.
     pub delete: bool,
     /// if operator supports delete with versioning.
-    pub delete_with_versioning: bool,
+    pub delete_with_version: bool,
 
     /// If operator supports copy.
     pub copy: bool,
@@ -141,7 +141,7 @@ pub struct Capability {
     /// If backend supports list with recursive.
     pub list_with_recursive: bool,
     /// if operator supports list with versioning.
-    pub list_with_versioning: bool,
+    pub list_with_version: bool,
 
     /// If operator supports presign.
     pub presign: bool,

core/tests/behavior/async_delete.rs

@@ -216,7 +216,7 @@ pub async fn test_remove_all_with_prefix_exists(op: Operator) -> Result<()> {
 }
 
 pub async fn test_delete_with_version(op: Operator) -> Result<()> {
-    if !op.info().full_capability().delete_with_versioning {
+    if !op.info().full_capability().delete_with_version {
         return Ok(());
     }
 
@@ -252,21 +252,32 @@ pub async fn test_delete_with_version(op: Operator) -> Result<()> {
 }
 
 pub async fn test_delete_with_not_existing_version(op: Operator) -> Result<()> {
-    if !op.info().full_capability().delete_with_versioning {
+    if !op.info().full_capability().delete_with_version {
         return Ok(());
     }
 
+    // retrieve a valid version
     let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(path.as_str(), content)
         .await
         .expect("write must success");
+    let version = op
+        .stat(path.as_str())
+        .await
+        .expect("stat must success")
+        .version()
+        .expect("must have stat")
+        .to_string();
 
+    let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
+    op.write(path.as_str(), content)
+        .await
+        .expect("write must success");
     let ret = op
         .delete_with(path.as_str())
-        .version("not-existing-version")
+        .version(version.as_str())
         .await;
-    assert!(ret.is_err());
-    assert_eq!(ret.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
+    assert!(ret.is_ok());
 
     Ok(())
 }

core/tests/behavior/async_list.rs

@@ -684,7 +684,7 @@ pub async fn test_list_only(op: Operator) -> Result<()> {
 }
 
 pub async fn test_list_files_with_version(op: Operator) -> Result<()> {
-    if !op.info().full_capability().list_with_versioning {
+    if !op.info().full_capability().list_with_version {
         return Ok(());
     }
 
@@ -722,7 +722,7 @@ pub async fn test_list_with_version_and_limit(op: Operator) -> Result<()> {
     if op.info().scheme() == Scheme::Gdrive {
         return Ok(());
     }
-    if !op.info().full_capability().list_with_versioning {
+    if !op.info().full_capability().list_with_version {
         return Ok(());
     }
 
@@ -775,7 +775,7 @@ pub async fn test_list_with_version_and_limit(op: Operator) -> Result<()> {
 }
 
 pub async fn test_list_with_version_and_start_after(op: Operator) -> Result<()> {
-    if !op.info().full_capability().list_with_versioning {
+    if !op.info().full_capability().list_with_version {
         return Ok(());
     }
 

core/tests/behavior/async_read.rs

@@ -557,7 +557,7 @@ pub async fn test_read_only_read_with_if_none_match(op: Operator) -> anyhow::Res
 }
 
 pub async fn test_read_with_version(op: Operator) -> anyhow::Result<()> {
-    if !op.info().full_capability().read_with_versioning {
+    if !op.info().full_capability().read_with_version {
         return Ok(());
     }
 
@@ -591,21 +591,30 @@ pub async fn test_read_with_version(op: Operator) -> anyhow::Result<()> {
 }
 
 pub async fn test_read_with_not_existing_version(op: Operator) -> anyhow::Result<()> {
-    if !op.info().full_capability().read_with_versioning {
+    if !op.info().full_capability().read_with_version {
         return Ok(());
     }
 
+    // retrieve a valid version
     let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(path.as_str(), content.clone())
         .await
         .expect("write must success");
+    let version = op
+        .stat(path.as_str())
+        .await
+        .expect("stat must success")
+        .version()
+        .expect("must have version")
+        .to_string();
 
-    let ret = op
-        .read_with(path.as_str())
-        .version("not-existing-version")
-        .await;
+    let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
+    op.write(path.as_str(), content)
+        .await
+        .expect("write must success");
+    let ret = op.read_with(path.as_str()).version(&version).await;
     assert!(ret.is_err());
-    assert_eq!(ret.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
+    assert_eq!(ret.unwrap_err().kind(), ErrorKind::NotFound);
 
     Ok(())
 }

core/tests/behavior/async_stat.rs

@@ -502,7 +502,7 @@ pub async fn test_read_only_stat_root(op: Operator) -> Result<()> {
 }
 
 pub async fn test_stat_with_version(op: Operator) -> Result<()> {
-    if !op.info().full_capability().stat_with_versioning {
+    if !op.info().full_capability().stat_with_version {
         return Ok(());
     }
 
@@ -540,21 +540,30 @@ pub async fn test_stat_with_version(op: Operator) -> Result<()> {
 }
 
 pub async fn stat_with_not_existing_version(op: Operator) -> Result<()> {
-    if !op.info().full_capability().stat_with_versioning {
+    if !op.info().full_capability().stat_with_version {
         return Ok(());
     }
 
+    // retrieve a valid version
     let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
     op.write(path.as_str(), content.clone())
         .await
         .expect("write must success");
+    let version = op
+        .stat(path.as_str())
+        .await
+        .expect("stat must success")
+        .version()
+        .expect("must have version")
+        .to_string();
 
-    let ret = op
-        .stat_with(path.as_str())
-        .version("not-existing-version")
-        .await;
+    let (path, content, _) = TEST_FIXTURE.new_file(op.clone());
+    op.write(path.as_str(), content)
+        .await
+        .expect("write must success");
+    let ret = op.stat_with(path.as_str()).version(version.as_str()).await;
     assert!(ret.is_err());
-    assert_eq!(ret.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
+    assert_eq!(ret.unwrap_err().kind(), ErrorKind::NotFound);
 
     Ok(())
 }