From d08d9c6a435258f1b4b885072d520f0f0751712a Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Wed, 6 Dec 2023 14:37:49 +0100 Subject: [PATCH 01/14] VTX-3411: fix serializatio for multi part field --- object_store/Cargo.toml | 2 +- object_store/src/aws/client.rs | 50 +++++++++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/object_store/Cargo.toml b/object_store/Cargo.toml index ff8047c60ca9..46f1413276b9 100644 --- a/object_store/Cargo.toml +++ b/object_store/Cargo.toml @@ -46,7 +46,7 @@ walkdir = "2" # Cloud storage support base64 = { version = "0.21", default-features = false, features = ["std"], optional = true } hyper = { version = "0.14", default-features = false, optional = true } -quick-xml = { version = "0.30.0", features = ["serialize", "overlapped-lists"], optional = true } +quick-xml = { git = "https://github.com/tafia/quick-xml.git", rev="db8546a", features = ["serialize", "overlapped-lists"], optional = true } serde = { version = "1.0", default-features = false, features = ["derive"], optional = true } serde_json = { version = "1.0", default-features = false, optional = true } rand = { version = "0.8", default-features = false, features = ["std", "std_rng"], optional = true } diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index 026792773253..e4eb55b89aa2 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -18,7 +18,8 @@ use crate::aws::checksum::Checksum; use crate::aws::credential::{AwsCredential, CredentialExt}; use crate::aws::{ - AwsCredentialProvider, S3CopyIfNotExists, STORE, STRICT_ENCODE_SET, STRICT_PATH_ENCODE_SET, + AwsCredentialProvider, S3CopyIfNotExists, STORE, STRICT_ENCODE_SET, + STRICT_PATH_ENCODE_SET, }; use crate::client::get::GetClient; use crate::client::list::ListClient; @@ -152,14 +153,24 @@ struct CompleteMultipart { part: Vec, } -#[derive(Debug, Serialize)] +#[derive(Debug)] struct MultipartPart { - #[serde(rename = "ETag")] e_tag: String, - #[serde(rename = "PartNumber")] part_number: usize, } +impl Serialize for MultipartPart { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let mut s = serializer.serialize_struct("MultipartPart", 2)?; + s.serialize_field("ETag", format!("\"{}\"", &self.e_tag).as_str())?; + s.serialize_field("PartNumber", &self.part_number)?; + s.end() + } +} + #[derive(Deserialize)] #[serde(rename_all = "PascalCase", rename = "DeleteResult")] struct BatchDeleteResponse { @@ -668,3 +679,34 @@ impl ListClient for S3Client { fn encode_path(path: &Path) -> PercentEncode<'_> { utf8_percent_encode(path.as_ref(), &STRICT_PATH_ENCODE_SET) } + +#[cfg(test)] +mod tests { + + #[test] + fn test_multipart_serializrtion() { + let request = CompleteMultipart { + part: vec![ + MultipartPart { + e_tag: "1".to_string(), + part_number: 1, + }, + MultipartPart { + e_tag: "2".to_string(), + part_number: 2, + }, + MultipartPart { + e_tag: "3".to_string(), + part_number: 3, + }, + ], + }; + + let body = quick_xml::se::to_string(&request).unwrap(); + + assert_eq!( + body, + r#""1"1"2"2"3"3"# + ) + } +} From da7a1177ea84cbb28c67620a9217030b9142ed08 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Wed, 6 Dec 2023 22:55:37 +0100 Subject: [PATCH 02/14] VTX-3411: imports --- object_store/src/aws/client.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index e4eb55b89aa2..115d8f7c34ae 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -42,6 +42,7 @@ use reqwest::{ header::{CONTENT_LENGTH, CONTENT_TYPE}, Client as ReqwestClient, Method, Response, StatusCode, }; +use serde::ser::SerializeStruct; use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; use std::collections::HashMap; @@ -682,6 +683,8 @@ fn encode_path(path: &Path) -> PercentEncode<'_> { #[cfg(test)] mod tests { + use crate::aws::client::{CompleteMultipart, MultipartPart}; + use quick_xml; #[test] fn test_multipart_serializrtion() { From 455ee53cc03f6ef4c6d73da62aba03232fad3609 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Wed, 6 Dec 2023 22:58:49 +0100 Subject: [PATCH 03/14] VTX-3411: fmt --- object_store/src/client/mod.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/object_store/src/client/mod.rs b/object_store/src/client/mod.rs index 534aab1d5a22..1bb9814bf89b 100644 --- a/object_store/src/client/mod.rs +++ b/object_store/src/client/mod.rs @@ -114,7 +114,7 @@ pub enum ClientConfigKey { /// User-Agent header to be used by this client UserAgent, /// PEM-formatted CA certificate - RootCa + RootCa, } impl AsRef for ClientConfigKey { @@ -160,11 +160,11 @@ impl FromStr for ClientConfigKey { "proxy_url" => Ok(Self::ProxyUrl), "timeout" => Ok(Self::Timeout), "user_agent" => Ok(Self::UserAgent), + "root_ca" => Ok(Self::RootCa), _ => Err(super::Error::UnknownConfigurationKey { store: "HTTP", key: s.into(), }), - "root_ca" => Ok(Self::RootCa), } } } @@ -238,9 +238,7 @@ impl ClientOptions { ClientConfigKey::UserAgent => { self.user_agent = Some(ConfigValue::Deferred(value.into())) } - ClientConfigKey::RootCa => { - self.root_ca = Some(value.into()) - } + ClientConfigKey::RootCa => self.root_ca = Some(value.into()), } self } @@ -282,9 +280,7 @@ impl ClientOptions { .as_ref() .and_then(|v| v.get().ok()) .and_then(|v| v.to_str().ok().map(|s| s.to_string())), - ClientConfigKey::RootCa => self - .root_ca - .clone(), + ClientConfigKey::RootCa => self.root_ca.clone(), } } @@ -437,7 +433,6 @@ impl ClientOptions { self } - /// Set a trusted CA certificate pub fn with_root_ca(mut self, root_ca: impl Into) -> Self { self.root_ca = Some(root_ca.into()); @@ -811,9 +806,7 @@ mod tests { user_agent ); assert_eq!( - builder - .get_config_value(&ClientConfigKey::RootCa) - .unwrap(), + builder.get_config_value(&ClientConfigKey::RootCa).unwrap(), root_ca ); } From ebd9446898b533fb9b670c0c5fdcf9b787f4287a Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Wed, 6 Dec 2023 23:01:42 +0100 Subject: [PATCH 04/14] VTX-3411: cleanup --- object_store/src/aws/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index 115d8f7c34ae..08103d36a687 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -687,7 +687,7 @@ mod tests { use quick_xml; #[test] - fn test_multipart_serializrtion() { + fn test_multipart_serialization() { let request = CompleteMultipart { part: vec![ MultipartPart { From 312ca666a2e928218e6fe38894d985a5c0106d4d Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 11:59:00 +0100 Subject: [PATCH 05/14] VTX-3411: update object store --- object_store/src/aws/client.rs | 2 +- object_store/src/gcp/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index 08103d36a687..0dd21264505b 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -165,7 +165,7 @@ impl Serialize for MultipartPart { where S: serde::Serializer, { - let mut s = serializer.serialize_struct("MultipartPart", 2)?; + let mut s = serializer.serialize_struct("Part", 2)?; s.serialize_field("ETag", format!("\"{}\"", &self.e_tag).as_str())?; s.serialize_field("PartNumber", &self.part_number)?; s.end() diff --git a/object_store/src/gcp/mod.rs b/object_store/src/gcp/mod.rs index d0737dcfbbd6..3f5bf629d180 100644 --- a/object_store/src/gcp/mod.rs +++ b/object_store/src/gcp/mod.rs @@ -567,7 +567,7 @@ impl PutPart for GCSMultipartUpload { #[async_trait] impl ObjectStore for GoogleCloudStorage { async fn put(&self, location: &Path, bytes: Bytes) -> Result<()> { - self.client.put_request(location, bytes, None).await + self.client.put_request(location, bytes).await } async fn put_multipart( From 0c6c26891ea362ecfec6af5b270bbea2b17639ee Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:08:21 +0100 Subject: [PATCH 06/14] VTX-3411: address clippy warnings --- object_store/src/azure/client.rs | 2 +- object_store/src/azure/credential.rs | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/object_store/src/azure/client.rs b/object_store/src/azure/client.rs index e18135c2c77c..cd1a3a10fcc7 100644 --- a/object_store/src/azure/client.rs +++ b/object_store/src/azure/client.rs @@ -372,7 +372,7 @@ struct ListResultInternal { } fn to_list_result(value: ListResultInternal, prefix: Option<&str>) -> Result { - let prefix = prefix.map(Path::from).unwrap_or_else(Path::default); + let prefix = prefix.map(Path::from).unwrap_or_default(); let common_prefixes = value .blobs .blob_prefix diff --git a/object_store/src/azure/credential.rs b/object_store/src/azure/credential.rs index fd75389249b0..c27e110e3fad 100644 --- a/object_store/src/azure/credential.rs +++ b/object_store/src/azure/credential.rs @@ -234,11 +234,8 @@ fn string_to_sign(h: &HeaderMap, u: &Url, method: &Method, account: &str) -> Str fn canonicalize_header(headers: &HeaderMap) -> String { let mut names = headers .iter() - .filter_map(|(k, _)| { - (k.as_str().starts_with("x-ms")) - // TODO remove unwraps - .then(|| (k.as_str(), headers.get(k).unwrap().to_str().unwrap())) - }) + .filter(|(k, _)| (k.as_str().starts_with("x-ms"))) + .map(|(k, _)| (k.as_str(), headers.get(k).unwrap().to_str().unwrap())) .collect::>(); names.sort_unstable(); From ce303aca88ee800ec806dca4c82363ec47329b01 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:12:55 +0100 Subject: [PATCH 07/14] VTX-3411: dead code --- object_store/src/client/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/object_store/src/client/mod.rs b/object_store/src/client/mod.rs index 1bb9814bf89b..385ccfa800d0 100644 --- a/object_store/src/client/mod.rs +++ b/object_store/src/client/mod.rs @@ -594,7 +594,9 @@ pub struct StaticCredentialProvider { } impl StaticCredentialProvider { + /// A [`CredentialProvider`] for a static credential of type `T` + #[allow(dead_code)] pub fn new(credential: T) -> Self { Self { credential: Arc::new(credential), From dd505d40d771e22c7a09207f8238d97bb4a375b7 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:15:34 +0100 Subject: [PATCH 08/14] VTX-3411: fmt --- object_store/src/aws/mod.rs | 9 +++++---- object_store/src/client/mod.rs | 1 - object_store/src/lib.rs | 5 ++++- object_store/src/limit.rs | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/object_store/src/aws/mod.rs b/object_store/src/aws/mod.rs index 71acdb3513a1..965d5dde18ca 100644 --- a/object_store/src/aws/mod.rs +++ b/object_store/src/aws/mod.rs @@ -214,7 +214,10 @@ impl AmazonS3 { #[async_trait] impl ObjectStore for AmazonS3 { async fn put(&self, location: &Path, bytes: Bytes) -> Result<()> { - self.client.put_request(location, bytes, &(), None).await.map(|_| ()) + self.client + .put_request(location, bytes, &(), None) + .await + .map(|_| ()) } async fn put_opts( @@ -224,9 +227,7 @@ impl ObjectStore for AmazonS3 { options: PutOptions, ) -> Result<()> { if options.tags.is_empty() { - self.client - .put_request(location, bytes, &(), None) - .await?; + self.client.put_request(location, bytes, &(), None).await?; } else { self.client .put_request(location, bytes, &(), Some(&options.tags)) diff --git a/object_store/src/client/mod.rs b/object_store/src/client/mod.rs index 385ccfa800d0..f4594e97692f 100644 --- a/object_store/src/client/mod.rs +++ b/object_store/src/client/mod.rs @@ -594,7 +594,6 @@ pub struct StaticCredentialProvider { } impl StaticCredentialProvider { - /// A [`CredentialProvider`] for a static credential of type `T` #[allow(dead_code)] pub fn new(credential: T) -> Self { diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs index 2044ac30acb4..000389764b71 100644 --- a/object_store/src/lib.rs +++ b/object_store/src/lib.rs @@ -273,7 +273,10 @@ pub mod throttle; mod client; #[cfg(feature = "cloud")] -pub use client::{backoff::BackoffConfig, retry::RetryConfig, CredentialProvider, ClientConfigKey, ClientOptions}; +pub use client::{ + backoff::BackoffConfig, retry::RetryConfig, ClientConfigKey, ClientOptions, + CredentialProvider, +}; use std::collections::HashMap; #[cfg(feature = "cloud")] diff --git a/object_store/src/limit.rs b/object_store/src/limit.rs index 9e6136a469e4..69b6e1a1587e 100644 --- a/object_store/src/limit.rs +++ b/object_store/src/limit.rs @@ -19,7 +19,7 @@ use crate::{ BoxStream, GetOptions, GetResult, GetResultPayload, ListResult, MultipartId, - ObjectMeta, ObjectStore, Path, PutOptions, Result, StreamExt, + ObjectMeta, ObjectStore, Path, PutOptions, Result, StreamExt, }; use async_trait::async_trait; use bytes::Bytes; From 074a00dbaa9bcdba1ddec066eb8cca3975d5dca7 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:18:29 +0100 Subject: [PATCH 09/14] VTX-3411: clippy --- object_store/src/aws/mod.rs | 6 ++---- object_store/src/lib.rs | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/object_store/src/aws/mod.rs b/object_store/src/aws/mod.rs index 965d5dde18ca..16703fb0768e 100644 --- a/object_store/src/aws/mod.rs +++ b/object_store/src/aws/mod.rs @@ -214,10 +214,8 @@ impl AmazonS3 { #[async_trait] impl ObjectStore for AmazonS3 { async fn put(&self, location: &Path, bytes: Bytes) -> Result<()> { - self.client - .put_request(location, bytes, &(), None) - .await - .map(|_| ()) + self.client.put_request(location, bytes, &(), None).await?; + Ok(()) } async fn put_opts( diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs index 000389764b71..bff0f83b5ab5 100644 --- a/object_store/src/lib.rs +++ b/object_store/src/lib.rs @@ -1258,7 +1258,6 @@ mod tests { let expected: Vec<_> = files .iter() - .cloned() .filter(|x| { let prefix_match = prefix.as_ref().map(|p| x.prefix_matches(p)).unwrap_or(true); From b98210f0e8377538d19567750a17fded68a1ae6c Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:19:36 +0100 Subject: [PATCH 10/14] VTX-3411: move clone into right position --- object_store/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs index bff0f83b5ab5..2e4009d62de4 100644 --- a/object_store/src/lib.rs +++ b/object_store/src/lib.rs @@ -1263,6 +1263,7 @@ mod tests { prefix.as_ref().map(|p| x.prefix_matches(p)).unwrap_or(true); prefix_match && x > &offset }) + .cloned() .collect(); assert_eq!(actual, expected, "{prefix:?} - {offset:?}"); From d2e5898b097faf6f83b316547a2dd729b35ed7a8 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:25:07 +0100 Subject: [PATCH 11/14] VTX-3411: clippy --- object_store/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs index 2e4009d62de4..a09360a0aa9b 100644 --- a/object_store/src/lib.rs +++ b/object_store/src/lib.rs @@ -1261,7 +1261,7 @@ mod tests { .filter(|x| { let prefix_match = prefix.as_ref().map(|p| x.prefix_matches(p)).unwrap_or(true); - prefix_match && x > &offset + prefix_match && (x > &offset) }) .cloned() .collect(); From 88dd767c52ada83d9237dd2b2224e8bba04777cf Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:36:18 +0100 Subject: [PATCH 12/14] VTX-3411: happy clippy --- object_store/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/object_store/src/lib.rs b/object_store/src/lib.rs index a09360a0aa9b..356dc3fee17b 100644 --- a/object_store/src/lib.rs +++ b/object_store/src/lib.rs @@ -1261,7 +1261,7 @@ mod tests { .filter(|x| { let prefix_match = prefix.as_ref().map(|p| x.prefix_matches(p)).unwrap_or(true); - prefix_match && (x > &offset) + prefix_match && (x > &&offset) }) .cloned() .collect(); From e46badea323592beda35a5c7ff688dbbddc0c560 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 12:52:08 +0100 Subject: [PATCH 13/14] VTX-3411: docs --- object_store/src/parse.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/object_store/src/parse.rs b/object_store/src/parse.rs index 1159e9a1af17..9b4896c5ac03 100644 --- a/object_store/src/parse.rs +++ b/object_store/src/parse.rs @@ -47,12 +47,12 @@ impl From for super::Error { } } -/// Recognises various URL formats, identifying the relevant [`ObjectStore`](crate::ObjectStore) +/// Recognises various URL formats, identifying the relevant [`ObjectStore`](ObjectStore) #[derive(Debug, Eq, PartialEq)] enum ObjectStoreScheme { - /// Url corresponding to [`LocalFileSystem`](crate::local::LocalFileSystem) + /// Url corresponding to [`LocalFileSystem`](LocalFileSystem) Local, - /// Url corresponding to [`InMemory`](crate::memory::InMemory) + /// Url corresponding to [`InMemory`](InMemory) Memory, /// Url corresponding to [`AmazonS3`](crate::aws::AmazonS3) AmazonS3, From 7611811f13ca87d13db7abad7b1eca47fe5a41a7 Mon Sep 17 00:00:00 2001 From: Faiaz Sanaulla Date: Fri, 8 Dec 2023 13:00:16 +0100 Subject: [PATCH 14/14] VTX-3411: docs --- object_store/src/parse.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/object_store/src/parse.rs b/object_store/src/parse.rs index 9b4896c5ac03..2e72a710ac75 100644 --- a/object_store/src/parse.rs +++ b/object_store/src/parse.rs @@ -47,12 +47,12 @@ impl From for super::Error { } } -/// Recognises various URL formats, identifying the relevant [`ObjectStore`](ObjectStore) +/// Recognises various URL formats, identifying the relevant [`ObjectStore`] #[derive(Debug, Eq, PartialEq)] enum ObjectStoreScheme { - /// Url corresponding to [`LocalFileSystem`](LocalFileSystem) + /// Url corresponding to [`LocalFileSystem`] Local, - /// Url corresponding to [`InMemory`](InMemory) + /// Url corresponding to [`InMemory`] Memory, /// Url corresponding to [`AmazonS3`](crate::aws::AmazonS3) AmazonS3,