From 4f2fc2adb296f00bea96f7d30813e3a4ac727366 Mon Sep 17 00:00:00 2001 From: Suyan Date: Thu, 27 Apr 2023 02:03:24 +0800 Subject: [PATCH] test(core): test for `write_with_cache_control` (#2131) * test for write_with_cache_control Signed-off-by: suyanhanx * disable gcs presign Signed-off-by: suyanhanx * enable for other cloud services Signed-off-by: suyanhanx * replace read with stat Signed-off-by: suyanhanx * replace presign with stat Signed-off-by: suyanhanx * restore presign Signed-off-by: suyanhanx * remove unused use Signed-off-by: suyanhanx * try fix Signed-off-by: suyanhanx * fix Signed-off-by: suyanhanx * try Signed-off-by: suyanhanx * try fix Signed-off-by: suyanhanx * debug Signed-off-by: suyanhanx * remove gcs cache-control support Signed-off-by: suyanhanx --------- Signed-off-by: suyanhanx --- core/src/services/azblob/backend.rs | 2 +- core/src/services/azblob/core.rs | 4 ++-- core/src/services/gcs/backend.rs | 9 +++++---- core/src/services/gcs/core.rs | 12 +---------- core/src/services/gcs/writer.rs | 1 - core/src/services/obs/backend.rs | 2 ++ core/src/services/oss/backend.rs | 1 + core/src/services/s3/backend.rs | 1 + core/src/services/s3/writer.rs | 2 +- core/tests/behavior/write.rs | 31 ++++++++++++++++++++++++++++- 10 files changed, 44 insertions(+), 21 deletions(-) diff --git a/core/src/services/azblob/backend.rs b/core/src/services/azblob/backend.rs index 4b7bd19223c8..69a59fe0a785 100644 --- a/core/src/services/azblob/backend.rs +++ b/core/src/services/azblob/backend.rs @@ -465,8 +465,8 @@ impl Accessor for AzblobBackend { read_with_override_content_disposition: true, write: true, - write_with_content_type: true, write_with_cache_control: true, + write_with_content_type: true, delete: true, create_dir: true, diff --git a/core/src/services/azblob/core.rs b/core/src/services/azblob/core.rs index 55f4b7d9043e..6f4e9b195c19 100644 --- a/core/src/services/azblob/core.rs +++ b/core/src/services/azblob/core.rs @@ -22,7 +22,6 @@ use std::fmt::Write; use std::str::FromStr; use http::header::HeaderName; -use http::header::CACHE_CONTROL; use http::header::CONTENT_LENGTH; use http::header::CONTENT_TYPE; use http::header::IF_MATCH; @@ -41,6 +40,7 @@ use crate::*; mod constants { pub const X_MS_BLOB_TYPE: &str = "x-ms-blob-type"; pub const X_MS_COPY_SOURCE: &str = "x-ms-copy-source"; + pub const X_MS_BLOB_CACHE_CONTROL: &str = "x-ms-blob-cache-control"; } pub struct AzblobCore { @@ -209,7 +209,7 @@ impl AzblobCore { let mut req = Request::put(&url); if let Some(cache_control) = cache_control { - req = req.header(CACHE_CONTROL, cache_control); + req = req.header(constants::X_MS_BLOB_CACHE_CONTROL, cache_control); } if let Some(size) = size { req = req.header(CONTENT_LENGTH, size) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index 5d01a534e9b1..7cfa47cf7f3e 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -398,9 +398,9 @@ impl Accessor for GcsBackend { } async fn create_dir(&self, path: &str, _: OpCreate) -> Result { - let mut req = - self.core - .gcs_insert_object_request(path, Some(0), None, None, AsyncBody::Empty)?; + let mut req = self + .core + .gcs_insert_object_request(path, Some(0), None, AsyncBody::Empty)?; self.core.sign(&mut req).await?; @@ -460,6 +460,7 @@ impl Accessor for GcsBackend { if resp.status().is_success() { // read http response body let slc = resp.into_body().bytes().await?; + let meta: GetObjectJsonResponse = serde_json::from_slice(&slc).map_err(new_json_deserialize_error)?; @@ -538,7 +539,7 @@ impl Accessor for GcsBackend { )?, PresignOperation::Write(_) => { self.core - .gcs_insert_object_xml_request(path, None, None, AsyncBody::Empty)? + .gcs_insert_object_xml_request(path, None, AsyncBody::Empty)? } }; diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 5a9d2bcf9dee..965d58baa1d7 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -24,7 +24,7 @@ use backon::ExponentialBuilder; use backon::Retryable; use bytes::Bytes; use bytes::BytesMut; -use http::header::CACHE_CONTROL; + use http::header::CONTENT_LENGTH; use http::header::CONTENT_RANGE; use http::header::CONTENT_TYPE; @@ -208,7 +208,6 @@ impl GcsCore { path: &str, size: Option, content_type: Option<&str>, - cache_control: Option<&str>, body: AsyncBody, ) -> Result> { let p = build_abs_path(&self.root, path); @@ -233,10 +232,6 @@ impl GcsCore { req = req.header(CONTENT_LENGTH, size.unwrap_or_default()); - if let Some(cache_control) = cache_control { - req = req.header(CACHE_CONTROL, cache_control) - } - if let Some(storage_class) = &self.default_storage_class { req = req.header(CONTENT_TYPE, "multipart/related; boundary=my-boundary"); @@ -276,7 +271,6 @@ impl GcsCore { &self, path: &str, content_type: Option<&str>, - cache_control: Option<&str>, body: AsyncBody, ) -> Result> { let p = build_abs_path(&self.root, path); @@ -289,10 +283,6 @@ impl GcsCore { req = req.header(CONTENT_TYPE, content_type); } - if let Some(cache_control) = cache_control { - req = req.header(CACHE_CONTROL, cache_control) - } - if let Some(acl) = &self.predefined_acl { req = req.header("x-goog-acl", acl); } diff --git a/core/src/services/gcs/writer.rs b/core/src/services/gcs/writer.rs index 1c48afbc231c..c4ed1f6e0449 100644 --- a/core/src/services/gcs/writer.rs +++ b/core/src/services/gcs/writer.rs @@ -67,7 +67,6 @@ impl GcsWriter { &percent_encode_path(&self.path), Some(bs.len()), self.op.content_type(), - self.op.cache_control(), AsyncBody::Bytes(bs), )?; diff --git a/core/src/services/obs/backend.rs b/core/src/services/obs/backend.rs index c9c77e004156..8a360f6199bf 100644 --- a/core/src/services/obs/backend.rs +++ b/core/src/services/obs/backend.rs @@ -314,6 +314,8 @@ impl Accessor for ObsBackend { read_can_next: true, write: true, + write_with_cache_control: true, + list: true, scan: true, copy: true, diff --git a/core/src/services/oss/backend.rs b/core/src/services/oss/backend.rs index 8ecb0c957134..a4284dc3fad2 100644 --- a/core/src/services/oss/backend.rs +++ b/core/src/services/oss/backend.rs @@ -432,6 +432,7 @@ impl Accessor for OssBackend { read_can_next: true, write: true, + write_with_cache_control: true, write_without_content_length: true, list: true, diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs index e0f2af72622b..44d8b5c6b129 100644 --- a/core/src/services/s3/backend.rs +++ b/core/src/services/s3/backend.rs @@ -924,6 +924,7 @@ impl Accessor for S3Backend { read_with_override_content_disposition: true, write: true, + write_with_cache_control: true, write_without_content_length: true, list: true, diff --git a/core/src/services/s3/writer.rs b/core/src/services/s3/writer.rs index aaf9e29124e9..93ee59a833c3 100644 --- a/core/src/services/s3/writer.rs +++ b/core/src/services/s3/writer.rs @@ -53,7 +53,7 @@ impl S3Writer { // The part size must be 5 MiB to 5 GiB. There is no minimum // size limit on the last part of your multipart upload. // - // We pick the default value as 8 MiB for better thoughput. + // We pick the default value as 8 MiB for better throughput. // // TODO: allow this value to be configured. buffer_size: 8 * 1024 * 1024, diff --git a/core/tests/behavior/write.rs b/core/tests/behavior/write.rs index 1b5a468ce3fd..31b4cfcd2580 100644 --- a/core/tests/behavior/write.rs +++ b/core/tests/behavior/write.rs @@ -21,7 +21,7 @@ use futures::AsyncSeekExt; use futures::StreamExt; use log::debug; use log::warn; -use opendal::ops::{OpRead, OpStat}; +use opendal::ops::{OpRead, OpStat, OpWrite}; use opendal::EntryMode; use opendal::ErrorKind; use opendal::Operator; @@ -75,6 +75,7 @@ macro_rules! behavior_write_tests { test_write, test_write_with_dir_path, test_write_with_special_chars, + test_write_with_cache_control, test_stat, test_stat_dir, test_stat_with_special_chars, @@ -179,6 +180,34 @@ pub async fn test_write_with_special_chars(op: Operator) -> Result<()> { Ok(()) } +// Write a single file with cache control should succeed. +pub async fn test_write_with_cache_control(op: Operator) -> Result<()> { + if !op.info().capability().write_with_cache_control { + return Ok(()); + } + + let path = uuid::Uuid::new_v4().to_string(); + let (content, _) = gen_bytes(); + + let target_cache_control = "no-cache, no-store, max-age=300"; + + let mut op_write = OpWrite::default(); + op_write = op_write.with_cache_control(target_cache_control); + + op.write_with(&path, op_write, content).await?; + + let meta = op.stat(&path).await.expect("stat must succeed"); + assert_eq!(meta.mode(), EntryMode::FILE); + assert_eq!( + meta.cache_control().expect("cache control must exist"), + target_cache_control + ); + + op.delete(&path).await.expect("delete must succeed"); + + Ok(()) +} + /// Stat existing file should return metadata pub async fn test_stat(op: Operator) -> Result<()> { let path = uuid::Uuid::new_v4().to_string();