From d569629719980cf4c3e2ecb08823e0678f2f1300 Mon Sep 17 00:00:00 2001
From: Marko Grujic <markoog@gmail.com>
Date: Sat, 4 Jan 2025 04:47:11 +0100
Subject: [PATCH] feat: Return hinted error for S3 wildcard if-none-match
 (#5506)

---
 core/src/layers/correctness_check.rs | 57 ++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/core/src/layers/correctness_check.rs b/core/src/layers/correctness_check.rs
index b3a08f8970fe..61d0669a393a 100644
--- a/core/src/layers/correctness_check.rs
+++ b/core/src/layers/correctness_check.rs
@@ -122,12 +122,16 @@ impl<A: Access> LayeredAccess for CorrectnessAccessor<A> {
                 "if_not_exists",
             ));
         }
-        if args.if_none_match().is_some() && !capability.write_with_if_none_match {
-            return Err(new_unsupported_error(
-                self.info.as_ref(),
-                Operation::Write,
-                "if_none_match",
-            ));
+        if let Some(if_none_match) = args.if_none_match() {
+            if !capability.write_with_if_none_match {
+                let mut err =
+                    new_unsupported_error(self.info.as_ref(), Operation::Write, "if_none_match");
+                if if_none_match == "*" && capability.write_with_if_not_exists {
+                    err = err.with_context("hint", "use if_not_exists instead");
+                }
+
+                return Err(err);
+            }
         }
 
         self.inner.write(path, args).await
@@ -304,7 +308,7 @@ mod tests {
         }
 
         async fn write(&self, _: &str, _: OpWrite) -> Result<(RpWrite, Self::Writer)> {
-            Ok((RpWrite::new(), Box::new(())))
+            Ok((RpWrite::new(), Box::new(MockWriter)))
         }
 
         async fn list(&self, _: &str, _: OpList) -> Result<(RpList, Self::Lister)> {
@@ -316,6 +320,22 @@ mod tests {
         }
     }
 
+    struct MockWriter;
+
+    impl oio::Write for MockWriter {
+        async fn write(&mut self, _: Buffer) -> Result<()> {
+            Ok(())
+        }
+
+        async fn close(&mut self) -> Result<()> {
+            Ok(())
+        }
+
+        async fn abort(&mut self) -> Result<()> {
+            Ok(())
+        }
+    }
+
     struct MockDeleter;
 
     impl oio::Delete for MockDeleter {
@@ -376,6 +396,7 @@ mod tests {
     async fn test_write_with() {
         let op = new_test_operator(Capability {
             write: true,
+            write_with_if_not_exists: true,
             ..Default::default()
         });
         let res = op.write_with("path", "".as_bytes()).append(true).await;
@@ -384,17 +405,31 @@ mod tests {
 
         let res = op
             .write_with("path", "".as_bytes())
-            .if_not_exists(true)
+            .if_none_match("etag")
             .await;
         assert!(res.is_err());
-        assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported);
+        assert_eq!(
+            res.unwrap_err().to_string(),
+            "Unsupported (permanent) at write => service memory doesn't support operation write with args if_none_match"
+        );
 
+        // Now try a wildcard if-none-match
         let res = op
             .write_with("path", "".as_bytes())
-            .if_none_match("etag")
+            .if_none_match("*")
             .await;
         assert!(res.is_err());
-        assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported);
+        assert_eq!(
+            res.unwrap_err().to_string(),
+            "Unsupported (permanent) at write, context: { hint: use if_not_exists instead } => \
+            service memory doesn't support operation write with args if_none_match"
+        );
+
+        let res = op
+            .write_with("path", "".as_bytes())
+            .if_not_exists(true)
+            .await;
+        assert!(res.is_ok());
 
         let op = new_test_operator(Capability {
             write: true,