From 5a5592c176c7a17f143d0f221b6495808b7f4054 Mon Sep 17 00:00:00 2001 From: Quentin Date: Wed, 11 Nov 2020 16:12:42 +0100 Subject: [PATCH 1/2] Replace with option syntaxic sugar --- src/api/s3_copy.rs | 20 ++++++-------------- src/api/s3_delete.rs | 21 +++++---------------- src/api/s3_get.rs | 35 +++++++++++------------------------ src/api/s3_list.rs | 7 +++---- src/api/s3_put.rs | 34 ++++++++++------------------------ 5 files changed, 35 insertions(+), 82 deletions(-) diff --git a/src/api/s3_copy.rs b/src/api/s3_copy.rs index 4280f4bf..5997e4fe 100644 --- a/src/api/s3_copy.rs +++ b/src/api/s3_copy.rs @@ -21,25 +21,20 @@ pub async fn handle_copy( source_bucket: &str, source_key: &str, ) -> Result, Error> { - let source_object = match garage + let source_object = garage .object_table .get(&source_bucket.to_string(), &source_key.to_string()) .await? - { - None => return Err(Error::NotFound), - Some(o) => o, - }; + .ok_or(Error::NotFound)?; - let source_last_v = match source_object + let source_last_v = source_object .versions() .iter() .rev() .filter(|v| v.is_complete()) .next() - { - Some(v) => v, - None => return Err(Error::NotFound), - }; + .ok_or(Error::NotFound)?; + let source_last_state = match &source_last_v.state { ObjectVersionState::Complete(x) => x, _ => unreachable!(), @@ -69,10 +64,7 @@ pub async fn handle_copy( .version_table .get(&source_last_v.uuid, &EmptyKey) .await?; - let source_version = match source_version { - Some(v) => v, - None => return Err(Error::NotFound), - }; + let source_version = source_version.ok_or(Error::NotFound)?; let dest_version = Version::new( new_uuid, diff --git a/src/api/s3_delete.rs b/src/api/s3_delete.rs index 33e47c17..f46bfcfe 100644 --- a/src/api/s3_delete.rs +++ b/src/api/s3_delete.rs @@ -16,17 +16,11 @@ async fn handle_delete_internal( bucket: &str, key: &str, ) -> Result<(UUID, UUID), Error> { - let object = match garage + let object = garage .object_table .get(&bucket.to_string(), &key.to_string()) .await? - { - None => { - // No need to delete - return Err(Error::NotFound); - } - Some(o) => o, - }; + .ok_or(Error::NotFound)?; // No need to delete let interesting_versions = object.versions().iter().filter(|v| match v.state { ObjectVersionState::Aborted => false, @@ -43,10 +37,7 @@ async fn handle_delete_internal( timestamp = std::cmp::max(timestamp, v.timestamp + 1); } - let deleted_version = match must_delete { - None => return Err(Error::NotFound), - Some(v) => v, - }; + let deleted_version = must_delete.ok_or(Error::NotFound)?; let version_uuid = gen_uuid(); @@ -142,10 +133,8 @@ fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Result del, - None => return Err(format!("Delete tag not found")), - }; + let delete = root.first_child().ok_or(format!("Delete tag not found"))?; + if !delete.has_tag_name("Delete") { return Err(format!("Invalid root tag: {:?}", root)); } diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index 71c656f2..a68c485b 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -41,25 +41,20 @@ pub async fn handle_head( bucket: &str, key: &str, ) -> Result, Error> { - let object = match garage + let object = garage .object_table .get(&bucket.to_string(), &key.to_string()) .await? - { - None => return Err(Error::NotFound), - Some(o) => o, - }; + .ok_or(Error::NotFound)?; - let version = match object + let version = object .versions() .iter() .rev() .filter(|v| v.is_data()) .next() - { - Some(v) => v, - None => return Err(Error::NotFound), - }; + .ok_or(Error::NotFound)?; + let version_meta = match &version.state { ObjectVersionState::Complete(ObjectVersionData::Inline(meta, _)) => meta, ObjectVersionState::Complete(ObjectVersionData::FirstBlock(meta, _)) => meta, @@ -80,25 +75,20 @@ pub async fn handle_get( bucket: &str, key: &str, ) -> Result, Error> { - let object = match garage + let object = garage .object_table .get(&bucket.to_string(), &key.to_string()) .await? - { - None => return Err(Error::NotFound), - Some(o) => o, - }; + .ok_or(Error::NotFound)?; - let last_v = match object + let last_v = object .versions() .iter() .rev() .filter(|v| v.is_complete()) .next() - { - Some(v) => v, - None => return Err(Error::NotFound), - }; + .ok_or(Error::NotFound)?; + let last_v_data = match &last_v.state { ObjectVersionState::Complete(x) => x, _ => unreachable!(), @@ -146,10 +136,7 @@ pub async fn handle_get( let get_next_blocks = garage.version_table.get(&last_v.uuid, &EmptyKey); let (first_block, version) = futures::try_join!(read_first_block, get_next_blocks)?; - let version = match version { - Some(v) => v, - None => return Err(Error::NotFound), - }; + let version = version.ok_or(Error::NotFound)?; let mut blocks = version .blocks() diff --git a/src/api/s3_list.rs b/src/api/s3_list.rs index 0a3b62ec..9e6bc0f5 100644 --- a/src/api/s3_list.rs +++ b/src/api/s3_list.rs @@ -64,10 +64,9 @@ pub async fn handle_list( } let common_prefix = if delimiter.len() > 0 { let relative_key = &object.key[prefix.len()..]; - match relative_key.find(delimiter) { - Some(i) => Some(&object.key[..prefix.len() + i + delimiter.len()]), - None => None, - } + relative_key + .find(delimiter) + .and_then(move |i| Some(&object.key[..prefix.len() + i + delimiter.len()])) } else { None }; diff --git a/src/api/s3_put.rs b/src/api/s3_put.rs index ea09524c..72613323 100644 --- a/src/api/s3_put.rs +++ b/src/api/s3_put.rs @@ -37,10 +37,7 @@ pub async fn handle_put( let body = req.into_body(); let mut chunker = BodyChunker::new(body, garage.config.block_size); - let first_block = match chunker.next().await? { - Some(x) => x, - None => vec![], - }; + let first_block = chunker.next().await?.unwrap_or(vec![]); let mut object_version = ObjectVersion { uuid: version_uuid, @@ -325,14 +322,9 @@ pub async fn handle_put_part( let (object, first_block) = futures::try_join!(get_object_fut, get_first_block_fut)?; // Check object is valid and multipart block can be accepted - let first_block = match first_block { - None => return Err(Error::BadRequest(format!("Empty body"))), - Some(x) => x, - }; - let object = match object { - None => return Err(Error::BadRequest(format!("Object not found"))), - Some(x) => x, - }; + let first_block = first_block.ok_or(Error::BadRequest(format!("Empty body")))?; + let object = object.ok_or(Error::BadRequest(format!("Object not found")))?; + if !object .versions() .iter() @@ -392,10 +384,8 @@ pub async fn handle_complete_multipart_upload( garage.object_table.get(&bucket, &key), garage.version_table.get(&version_uuid, &EmptyKey), )?; - let object = match object { - None => return Err(Error::BadRequest(format!("Object not found"))), - Some(x) => x, - }; + let object = object.ok_or(Error::BadRequest(format!("Object not found")))?; + let object_version = object .versions() .iter() @@ -408,10 +398,8 @@ pub async fn handle_complete_multipart_upload( } Some(x) => x.clone(), }; - let version = match version { - None => return Err(Error::BadRequest(format!("Version not found"))), - Some(x) => x, - }; + let version = version.ok_or(Error::BadRequest(format!("Version not found")))?; + if version.blocks().len() == 0 { return Err(Error::BadRequest(format!("No data was uploaded"))); } @@ -474,10 +462,8 @@ pub async fn handle_abort_multipart_upload( .object_table .get(&bucket.to_string(), &key.to_string()) .await?; - let object = match object { - None => return Err(Error::BadRequest(format!("Object not found"))), - Some(x) => x, - }; + let object = object.ok_or(Error::BadRequest(format!("Object not found")))?; + let object_version = object .versions() .iter() From 7d7b9e95a9b7fe71e7bb42b08a22698475a9c78c Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 11 Nov 2020 16:36:48 +0100 Subject: [PATCH 2/2] Simplify and_then(Some) as map() and remove move --- src/api/s3_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/s3_list.rs b/src/api/s3_list.rs index 9e6bc0f5..f2b49a1d 100644 --- a/src/api/s3_list.rs +++ b/src/api/s3_list.rs @@ -66,7 +66,7 @@ pub async fn handle_list( let relative_key = &object.key[prefix.len()..]; relative_key .find(delimiter) - .and_then(move |i| Some(&object.key[..prefix.len() + i + delimiter.len()])) + .map(|i| &object.key[..prefix.len() + i + delimiter.len()]) } else { None };