Support ETag in InMemory (apache#4879)

object_store/src/lib.rs

@@ -718,25 +718,41 @@ pub struct GetOptions {
 
 impl GetOptions {
     /// Returns an error if the modification conditions on this request are not satisfied
-    fn check_modified(
-        &self,
-        location: &Path,
-        last_modified: DateTime<Utc>,
-    ) -> Result<()> {
-        if let Some(date) = self.if_modified_since {
-            if last_modified <= date {
-                return Err(Error::NotModified {
-                    path: location.to_string(),
-                    source: format!("{} >= {}", date, last_modified).into(),
+    ///
+    /// <https://datatracker.ietf.org/doc/html/rfc7232#section-6>
+    fn test(&self, meta: &ObjectMeta) -> Result<()> {
+        // The use of the invalid etag "*" means no ETag is equivalent to never matching
+        let etag = meta.e_tag.as_deref().unwrap_or("*");
+        let last_modified = meta.last_modified;
+
+        if let Some(m) = &self.if_match {
+            if m != "*" && m.split(',').map(str::trim).all(|x| x != etag) {
+                return Err(Error::Precondition {
+                    path: meta.location.to_string(),
+                    source: format!("{etag} does not match {m}").into(),
                 });
             }
-        }
-
-        if let Some(date) = self.if_unmodified_since {
+        } else if let Some(date) = self.if_unmodified_since {
             if last_modified > date {
                 return Err(Error::Precondition {
-                    path: location.to_string(),
-                    source: format!("{} < {}", date, last_modified).into(),
+                    path: meta.location.to_string(),
+                    source: format!("{date} < {last_modified}").into(),
+                });
+            }
+        }
+
+        if let Some(m) = &self.if_none_match {
+            if m == "*" || m.split(',').map(str::trim).any(|x| x == etag) {
+                return Err(Error::NotModified {
+                    path: meta.location.to_string(),
+                    source: format!("{etag} matches {m}").into(),
+                });
+            }
+        } else if let Some(date) = self.if_modified_since {
+            if last_modified <= date {
+                return Err(Error::NotModified {
+                    path: meta.location.to_string(),
+                    source: format!("{date} >= {last_modified}").into(),
                 });
             }
         }
@@ -940,6 +956,7 @@ mod test_util {
 mod tests {
     use super::*;
     use crate::test_util::flatten_list_stream;
+    use chrono::TimeZone;
     use rand::{thread_rng, Rng};
     use tokio::io::AsyncWriteExt;
 
@@ -1685,8 +1702,86 @@ mod tests {
         assert!(stream.next().await.is_none());
     }
 
-    // Tests TODO:
-    // GET nonexisting location (in_memory/file)
-    // DELETE nonexisting location
-    // PUT overwriting
+    #[test]
+    fn test_get_options() {
+        let mut meta = ObjectMeta {
+            location: Path::from("test"),
+            last_modified: Utc.timestamp_nanos(100),
+            size: 100,
+            e_tag: Some("123".to_string()),
+        };
+
+        let mut options = GetOptions::default();
+        options.test(&meta).unwrap();
+
+        options.if_modified_since = Some(Utc.timestamp_nanos(50));
+        options.test(&meta).unwrap();
+
+        options.if_modified_since = Some(Utc.timestamp_nanos(100));
+        options.test(&meta).unwrap_err();
+
+        options.if_modified_since = Some(Utc.timestamp_nanos(101));
+        options.test(&meta).unwrap_err();
+
+        options = GetOptions::default();
+
+        options.if_unmodified_since = Some(Utc.timestamp_nanos(50));
+        options.test(&meta).unwrap_err();
+
+        options.if_unmodified_since = Some(Utc.timestamp_nanos(100));
+        options.test(&meta).unwrap();
+
+        options.if_unmodified_since = Some(Utc.timestamp_nanos(101));
+        options.test(&meta).unwrap();
+
+        options = GetOptions::default();
+
+        options.if_match = Some("123".to_string());
+        options.test(&meta).unwrap();
+
+        options.if_match = Some("123,354".to_string());
+        options.test(&meta).unwrap();
+
+        options.if_match = Some("354, 123,".to_string());
+        options.test(&meta).unwrap();
+
+        options.if_match = Some("354".to_string());
+        options.test(&meta).unwrap_err();
+
+        options.if_match = Some("*".to_string());
+        options.test(&meta).unwrap();
+
+        // If-Match takes precedence
+        options.if_unmodified_since = Some(Utc.timestamp_nanos(200));
+        options.test(&meta).unwrap();
+
+        options = GetOptions::default();
+
+        options.if_none_match = Some("123".to_string());
+        options.test(&meta).unwrap_err();
+
+        options.if_none_match = Some("*".to_string());
+        options.test(&meta).unwrap_err();
+
+        options.if_none_match = Some("1232".to_string());
+        options.test(&meta).unwrap();
+
+        options.if_none_match = Some("23, 123".to_string());
+        options.test(&meta).unwrap_err();
+
+        // If-None-Match takes precedence
+        options.if_modified_since = Some(Utc.timestamp_nanos(10));
+        options.test(&meta).unwrap_err();
+
+        // Check missing ETag
+        meta.e_tag = None;
+        options = GetOptions::default();
+
+        options.if_none_match = Some("*".to_string()); // Fails if any file exists
+        options.test(&meta).unwrap_err();
+
+        options = GetOptions::default();
+        options.if_match = Some("*".to_string()); // Passes if file exists
+        options.test(&meta).unwrap();
+    }
 }

object_store/src/local.rs

@@ -365,23 +365,13 @@ impl ObjectStore for LocalFileSystem {
     }
 
     async fn get_opts(&self, location: &Path, options: GetOptions) -> Result<GetResult> {
-        if options.if_match.is_some() || options.if_none_match.is_some() {
-            return Err(super::Error::NotSupported {
-                source: "ETags not supported by LocalFileSystem".to_string().into(),
-            });
-        }
-
         let location = location.clone();
         let path = self.config.path_to_filesystem(&location)?;
         maybe_spawn_blocking(move || {
             let (file, metadata) = open_file(&path)?;
-            if options.if_unmodified_since.is_some()
-                || options.if_modified_since.is_some()
-            {
-                options.check_modified(&location, last_modified(&metadata))?;
-            }
 
             let meta = convert_metadata(metadata, location)?;
+            options.test(&meta)?;
 
             Ok(GetResult {
                 payload: GetResultPayload::File(file, path),