fix cr comments

core/src/raw/chrono_util.rs

@@ -63,9 +63,24 @@ pub fn parse_datetime_from_from_timestamp(s: i64) -> Result<DateTime<Utc>> {
     Ok(st.into())
 }
 
-/// format datetime to rfc1123
+/// format datetime into http date, this format is required by:
+/// https://httpwg.org/specs/rfc9110.html#field.if-modified-since
 ///
-/// for example: `Sun, 06 Nov 1994 08:49:37 GMT`
-pub fn to_rfc_1123(s: DateTime<Utc>) -> String {
+pub fn format_datetime_into_http_date(s: DateTime<Utc>) -> String {
     s.format("%a, %d %b %Y %H:%M:%S GMT").to_string()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_format_datetime_into_http_date() {
+        let s = "Sat, 29 Oct 1994 19:43:31 +0000";
+        let v = parse_datetime_from_rfc2822(s).unwrap();
+        assert_eq!(
+            format_datetime_into_http_date(v),
+            "Sat, 29 Oct 1994 19:43:31 GMT"
+        );
+    }
+}

core/src/raw/http_util/body.rs

@@ -39,7 +39,7 @@ pub struct HttpBody {
 
 /// # Safety
 ///
-/// HttpBody is sent on non wasm32 targets.
+/// HttpBody is `Send` on non wasm32 targets.
 unsafe impl Send for HttpBody {}
 
 /// # Safety

core/src/raw/ops.rs

@@ -299,13 +299,13 @@ pub struct OpRead {
     range: BytesRange,
     if_match: Option<String>,
     if_none_match: Option<String>,
+    if_modified_since: Option<DateTime<Utc>>,
+    if_unmodified_since: Option<DateTime<Utc>>,
     override_content_type: Option<String>,
     override_cache_control: Option<String>,
     override_content_disposition: Option<String>,
     version: Option<String>,
     executor: Option<Executor>,
-    if_modified_since: Option<DateTime<Utc>>,
-    if_unmodified_since: Option<DateTime<Utc>>,
 }
 
 impl OpRead {
@@ -386,6 +386,28 @@ impl OpRead {
         self.if_none_match.as_deref()
     }
 
+    /// Set the If-Modified-Since of the option
+    pub fn with_if_modified_since(mut self, v: DateTime<Utc>) -> Self {
+        self.if_modified_since = Some(v);
+        self
+    }
+
+    /// Get If-Modified-Since from option
+    pub fn if_modified_since(&self) -> Option<DateTime<Utc>> {
+        self.if_modified_since
+    }
+
+    /// Set the If-Unmodified-Since of the option
+    pub fn with_if_unmodified_since(mut self, v: DateTime<Utc>) -> Self {
+        self.if_unmodified_since = Some(v);
+        self
+    }
+
+    /// Get If-Unmodified-Since from option
+    pub fn if_unmodified_since(&self) -> Option<DateTime<Utc>> {
+        self.if_unmodified_since
+    }
+
     /// Set the version of the option
     pub fn with_version(mut self, version: &str) -> Self {
         self.version = Some(version.to_string());
@@ -421,28 +443,6 @@ impl OpRead {
     pub fn executor(&self) -> Option<&Executor> {
         self.executor.as_ref()
     }
-
-    /// Set the If-Modified-Since of the option
-    pub fn with_if_modified_since(mut self, if_modified_since: DateTime<Utc>) -> Self {
-        self.if_modified_since = Some(if_modified_since);
-        self
-    }
-
-    /// Get If-Modified-Since from option
-    pub fn if_modified_since(&self) -> Option<DateTime<Utc>> {
-        self.if_modified_since
-    }
-
-    /// Set the If-Unmodified-Since of the option
-    pub fn with_if_unmodified_since(mut self, if_unmodified_since: DateTime<Utc>) -> Self {
-        self.if_unmodified_since = Some(if_unmodified_since);
-        self
-    }
-
-    /// Get If-Unmodified-Since from option
-    pub fn if_unmodified_since(&self) -> Option<DateTime<Utc>> {
-        self.if_unmodified_since
-    }
 }
 
 /// Args for reader operation.

core/src/services/s3/backend.rs

@@ -944,12 +944,12 @@ impl Access for S3Backend {
                 read: true,
                 read_with_if_match: true,
                 read_with_if_none_match: true,
+                read_with_if_modified_since: true,
+                read_with_if_unmodified_since: true,
                 read_with_override_cache_control: true,
                 read_with_override_content_disposition: true,
                 read_with_override_content_type: true,
                 read_with_version: self.core.enable_versioning,
-                read_with_if_modified_since: true,
-                read_with_if_unmodified_since: true,
 
                 write: true,
                 write_can_empty: true,

core/src/services/s3/core.rs

@@ -408,11 +408,17 @@ impl S3Core {
         }
 
         if let Some(if_modified_since) = args.if_modified_since() {
-            req = req.header(IF_MODIFIED_SINCE, to_rfc_1123(if_modified_since));
+            req = req.header(
+                IF_MODIFIED_SINCE,
+                format_datetime_into_http_date(if_modified_since),
+            );
         }
 
         if let Some(if_unmodified_since) = args.if_unmodified_since() {
-            req = req.header(IF_UNMODIFIED_SINCE, to_rfc_1123(if_unmodified_since));
+            req = req.header(
+                IF_UNMODIFIED_SINCE,
+                format_datetime_into_http_date(if_unmodified_since),
+            );
         }
 
         // Set SSE headers.

core/src/types/capability.rs

@@ -107,6 +107,10 @@ pub struct Capability {
     pub read_with_if_match: bool,
     /// Indicates if conditional read operations using If-None-Match are supported.
     pub read_with_if_none_match: bool,
+    /// Indicates if conditional read operations using If-Modified-Since are supported.
+    pub read_with_if_modified_since: bool,
+    /// Indicates if conditional read operations using If-Unmodified-Since are supported.
+    pub read_with_if_unmodified_since: bool,
     /// Indicates if Cache-Control header override is supported during read operations.
     pub read_with_override_cache_control: bool,
     /// Indicates if Content-Disposition header override is supported during read operations.
@@ -115,10 +119,6 @@ pub struct Capability {
     pub read_with_override_content_type: bool,
     /// Indicates if versions read operations are supported.
     pub read_with_version: bool,
-    /// Indicates if conditional read operations using If-Modified-Since are supported.
-    pub read_with_if_modified_since: bool,
-    /// Indicates if conditional read operations using If-Unmodified-Since are supported.
-    pub read_with_if_unmodified_since: bool,
 
     /// Indicates if the operator supports write operations.
     pub write: bool,

core/src/types/operator/operator_futures.rs

@@ -212,6 +212,16 @@ impl<F: Future<Output = Result<Buffer>>> FutureRead<F> {
         self.map(|(args, op_reader)| (args.with_if_none_match(v), op_reader))
     }
 
+    /// Set the If-Modified-Since for this operation.
+    pub fn if_modified_since(self, v: DateTime<Utc>) -> Self {
+        self.map(|(args, op_reader)| (args.with_if_modified_since(v), op_reader))
+    }
+
+    /// Set the If-Unmodified-Since for this operation.
+    pub fn if_unmodified_since(self, v: DateTime<Utc>) -> Self {
+        self.map(|(args, op_reader)| (args.with_if_unmodified_since(v), op_reader))
+    }
+
     /// Set the version for this operation.
     pub fn version(self, v: &str) -> Self {
         self.map(|(args, op_reader)| (args.with_version(v), op_reader))
@@ -236,16 +246,6 @@ impl<F: Future<Output = Result<Buffer>>> FutureRead<F> {
     pub fn chunk(self, chunk_size: usize) -> Self {
         self.map(|(args, op_reader)| (args, op_reader.with_chunk(chunk_size)))
     }
-
-    /// Set the If-Modified-Since for this operation.
-    pub fn if_modified_since(self, v: DateTime<Utc>) -> Self {
-        self.map(|(args, op_reader)| (args.with_if_modified_since(v), op_reader))
-    }
-
-    /// Set the If-Unmodified-Since for this operation.
-    pub fn if_unmodified_since(self, v: DateTime<Utc>) -> Self {
-        self.map(|(args, op_reader)| (args.with_if_unmodified_since(v), op_reader))
-    }
 }
 
 /// Future that generated by [`Operator::read_with`] or [`Operator::reader_with`].
@@ -268,6 +268,16 @@ impl<F: Future<Output = Result<Reader>>> FutureReader<F> {
         self.map(|(op_read, op_reader)| (op_read.with_if_none_match(etag), op_reader))
     }
 
+    /// Set the If-Modified-Since for this operation.
+    pub fn if_modified_since(self, v: DateTime<Utc>) -> Self {
+        self.map(|(op_read, op_reader)| (op_read.with_if_modified_since(v), op_reader))
+    }
+
+    /// Set the If-Unmodified-Since for this operation.
+    pub fn if_unmodified_since(self, v: DateTime<Utc>) -> Self {
+        self.map(|(op_read, op_reader)| (op_read.with_if_unmodified_since(v), op_reader))
+    }
+
     /// Set the version for this operation.
     pub fn version(self, v: &str) -> Self {
         self.map(|(op_read, op_reader)| (op_read.with_version(v), op_reader))
@@ -287,16 +297,6 @@ impl<F: Future<Output = Result<Reader>>> FutureReader<F> {
     pub fn gap(self, gap_size: usize) -> Self {
         self.map(|(op_read, op_reader)| (op_read, op_reader.with_gap(gap_size)))
     }
-
-    /// Set the If-Modified-Since for this operation.
-    pub fn if_modified_since(self, v: DateTime<Utc>) -> Self {
-        self.map(|(op_read, op_reader)| (op_read.with_if_modified_since(v), op_reader))
-    }
-
-    /// Set the If-Unmodified-Since for this operation.
-    pub fn if_unmodified_since(self, v: DateTime<Utc>) -> Self {
-        self.map(|(op_read, op_reader)| (op_read.with_if_unmodified_since(v), op_reader))
-    }
 }
 
 /// Future that generated by [`Operator::write_with`].

core/tests/behavior/async_read.rs

@@ -44,15 +44,15 @@ pub fn tests(op: &Operator, tests: &mut Vec<Trial>) {
             test_read_not_exist,
             test_read_with_if_match,
             test_read_with_if_none_match,
+            test_read_with_if_modified_since,
+            test_read_with_if_unmodified_since,
             test_read_with_dir_path,
             test_read_with_special_chars,
             test_read_with_override_cache_control,
             test_read_with_override_content_disposition,
             test_read_with_override_content_type,
             test_read_with_version,
-            test_read_with_not_existing_version,
-            test_read_with_if_modified_since,
-            test_read_with_if_unmodified_since
+            test_read_with_not_existing_version
         ))
     }
 
@@ -286,21 +286,15 @@ pub async fn test_reader_with_if_modified_since(op: Operator) -> anyhow::Result<
         .await
         .expect("write must succeed");
 
-    let one_hour_ago_check = chrono::Utc::now() - chrono::Duration::seconds(3600);
-    let reader = op
-        .reader_with(&path)
-        .if_modified_since(one_hour_ago_check)
-        .await?;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(3600);
+    let reader = op.reader_with(&path).if_modified_since(since).await?;
     let bs = reader.read(..).await?.to_bytes();
     assert_eq!(bs, content);
 
     sleep(Duration::from_secs(3)).await;
 
-    let one_second_ago_check = chrono::Utc::now() - chrono::Duration::seconds(1);
-    let reader = op
-        .reader_with(&path)
-        .if_modified_since(one_second_ago_check)
-        .await?;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(1);
+    let reader = op.reader_with(&path).if_modified_since(since).await?;
     let res = reader.read(..).await;
     assert!(res.is_err());
     assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
@@ -320,22 +314,16 @@ pub async fn test_reader_with_if_unmodified_since(op: Operator) -> anyhow::Resul
         .await
         .expect("write must succeed");
 
-    let one_hour_ago_check = chrono::Utc::now() - chrono::Duration::seconds(3600);
-    let reader = op
-        .reader_with(&path)
-        .if_unmodified_since(one_hour_ago_check)
-        .await?;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(3600);
+    let reader = op.reader_with(&path).if_unmodified_since(since).await?;
     let res = reader.read(..).await;
     assert!(res.is_err());
     assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
 
     sleep(Duration::from_secs(3)).await;
 
-    let one_second_ago_check = chrono::Utc::now() - chrono::Duration::seconds(1);
-    let reader = op
-        .reader_with(&path)
-        .if_unmodified_since(one_second_ago_check)
-        .await?;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(1);
+    let reader = op.reader_with(&path).if_unmodified_since(since).await?;
     let bs = reader.read(..).await?.to_bytes();
     assert_eq!(bs, content);
 
@@ -576,21 +564,18 @@ pub async fn test_read_with_if_modified_since(op: Operator) -> anyhow::Result<()
         .await
         .expect("write must succeed");
 
-    let one_hour_ago_check = chrono::Utc::now() - chrono::Duration::seconds(3600);
+    let since = chrono::Utc::now() - chrono::Duration::seconds(3600);
     let bs = op
         .read_with(&path)
-        .if_modified_since(one_hour_ago_check)
+        .if_modified_since(since)
         .await?
         .to_bytes();
     assert_eq!(bs, content);
 
     sleep(Duration::from_secs(3)).await;
 
-    let one_second_ago_check = chrono::Utc::now() - chrono::Duration::seconds(1);
-    let res = op
-        .read_with(&path)
-        .if_modified_since(one_second_ago_check)
-        .await;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(1);
+    let res = op.read_with(&path).if_modified_since(since).await;
     assert!(res.is_err());
     assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
 
@@ -609,20 +594,17 @@ pub async fn test_read_with_if_unmodified_since(op: Operator) -> anyhow::Result<
         .await
         .expect("write must succeed");
 
-    let one_hour_ago_check = chrono::Utc::now() - chrono::Duration::seconds(3600);
-    let res = op
-        .read_with(&path)
-        .if_unmodified_since(one_hour_ago_check)
-        .await;
+    let since = chrono::Utc::now() - chrono::Duration::seconds(3600);
+    let res = op.read_with(&path).if_unmodified_since(since).await;
     assert!(res.is_err());
     assert_eq!(res.unwrap_err().kind(), ErrorKind::ConditionNotMatch);
 
     sleep(Duration::from_secs(3)).await;
 
-    let one_second_ago_check = chrono::Utc::now() - chrono::Duration::seconds(1);
+    let since = chrono::Utc::now() - chrono::Duration::seconds(1);
     let bs = op
         .read_with(&path)
-        .if_unmodified_since(one_second_ago_check)
+        .if_unmodified_since(since)
         .await?
         .to_bytes();
     assert_eq!(bs, content);