From d19edd9a2c51095c093f4af1f3003a2f392c803e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Mon, 16 Feb 2026 11:50:07 +0800 Subject: [PATCH] refactor(storage): use named params for multipart list APIs (#1833) --- rustfs/src/storage/ecfs.rs | 24 +++++++-- rustfs/src/storage/s3_api/multipart.rs | 70 ++++++++++++++++++-------- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/rustfs/src/storage/ecfs.rs b/rustfs/src/storage/ecfs.rs index 8f5c51f56..fb4e42a2a 100644 --- a/rustfs/src/storage/ecfs.rs +++ b/rustfs/src/storage/ecfs.rs @@ -3574,14 +3574,21 @@ impl S3 for FS { return Err(not_initialized_error()); }; - let (prefix, key_marker, max_uploads) = parse_list_multipart_uploads_params(prefix, key_marker, max_uploads)?; + let parsed = parse_list_multipart_uploads_params(prefix, key_marker, max_uploads)?; let result = store - .list_multipart_uploads(&bucket, &prefix, delimiter, key_marker, upload_id_marker, max_uploads) + .list_multipart_uploads( + &bucket, + &parsed.prefix, + delimiter, + parsed.key_marker, + upload_id_marker, + parsed.max_uploads, + ) .await .map_err(ApiError::from)?; - let output = build_list_multipart_uploads_output(bucket, prefix, result); + let output = build_list_multipart_uploads_output(bucket, parsed.prefix, result); Ok(s3_response(output)) } @@ -3708,10 +3715,17 @@ impl S3 for FS { return Err(not_initialized_error()); }; - let (part_number_marker, max_parts) = parse_list_parts_params(part_number_marker, max_parts)?; + let parsed = parse_list_parts_params(part_number_marker, max_parts)?; let res = store - .list_object_parts(&bucket, &key, &upload_id, part_number_marker, max_parts, &ObjectOptions::default()) + .list_object_parts( + &bucket, + &key, + &upload_id, + parsed.part_number_marker, + parsed.max_parts, + &ObjectOptions::default(), + ) .await .map_err(ApiError::from)?; diff --git a/rustfs/src/storage/s3_api/multipart.rs b/rustfs/src/storage/s3_api/multipart.rs index 1f4409187..6dabf0a70 100644 --- a/rustfs/src/storage/s3_api/multipart.rs +++ b/rustfs/src/storage/s3_api/multipart.rs @@ -19,6 +19,19 @@ use rustfs_ecstore::store_api::{ListMultipartsInfo, ListPartsInfo}; use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp}; use s3s::{S3Error, S3ErrorCode}; +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct ListPartsParams { + pub part_number_marker: Option, + pub max_parts: usize, +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct ListMultipartUploadsParams { + pub prefix: String, + pub key_marker: Option, + pub max_uploads: usize, +} + pub(crate) fn build_list_parts_output(res: ListPartsInfo) -> ListPartsOutput { let owner = rustfs_owner(); let initiator = rustfs_initiator(); @@ -57,8 +70,19 @@ pub(crate) fn build_list_parts_output(res: ListPartsInfo) -> ListPartsOutput { pub(crate) fn parse_list_parts_params( part_number_marker: Option, max_parts: Option, -) -> Result<(Option, usize), S3Error> { - let part_number_marker = part_number_marker.map(|x| x as usize); +) -> Result { + let part_number_marker = match part_number_marker { + Some(marker) => { + if marker < 0 { + return Err(S3Error::with_message( + S3ErrorCode::InvalidArgument, + "part-number-marker must be non-negative".to_string(), + )); + } + Some(marker as usize) + } + None => None, + }; let max_parts = match max_parts { Some(parts) => { if !(1..=1000).contains(&parts) { @@ -72,14 +96,17 @@ pub(crate) fn parse_list_parts_params( None => 1000, }; - Ok((part_number_marker, max_parts)) + Ok(ListPartsParams { + part_number_marker, + max_parts, + }) } pub(crate) fn parse_list_multipart_uploads_params( prefix: Option, key_marker: Option, max_uploads: Option, -) -> Result<(String, Option, usize), S3Error> { +) -> Result { let prefix = prefix.unwrap_or_default(); let max_uploads = match max_uploads { Some(value) => { @@ -108,7 +135,11 @@ pub(crate) fn parse_list_multipart_uploads_params( return Err(S3Error::with_message(S3ErrorCode::NotImplemented, "Invalid key marker".to_string())); } - Ok((prefix, key_marker, max_uploads)) + Ok(ListMultipartUploadsParams { + prefix, + key_marker, + max_uploads, + }) } pub(crate) fn build_list_multipart_uploads_output( @@ -269,13 +300,13 @@ mod tests { #[test] fn test_parse_list_parts_params_defaults_and_valid_values() { - let (part_number_marker, max_parts) = parse_list_parts_params(Some(5), Some(100)).expect("expected valid params"); - assert_eq!(part_number_marker, Some(5)); - assert_eq!(max_parts, 100); + let parsed = parse_list_parts_params(Some(5), Some(100)).expect("expected valid params"); + assert_eq!(parsed.part_number_marker, Some(5)); + assert_eq!(parsed.max_parts, 100); - let (part_number_marker, max_parts) = parse_list_parts_params(None, None).expect("expected default params"); - assert_eq!(part_number_marker, None); - assert_eq!(max_parts, 1000); + let parsed = parse_list_parts_params(None, None).expect("expected default params"); + assert_eq!(parsed.part_number_marker, None); + assert_eq!(parsed.max_parts, 1000); } #[test] @@ -289,18 +320,17 @@ mod tests { #[test] fn test_parse_list_multipart_uploads_params_defaults_and_valid_values() { - let (prefix, key_marker, max_uploads) = + let parsed = parse_list_multipart_uploads_params(Some("prefix/".to_string()), Some("prefix/key-marker".to_string()), Some(100)) .expect("expected valid params"); - assert_eq!(prefix, "prefix/"); - assert_eq!(key_marker.as_deref(), Some("prefix/key-marker")); - assert_eq!(max_uploads, 100); + assert_eq!(parsed.prefix, "prefix/"); + assert_eq!(parsed.key_marker.as_deref(), Some("prefix/key-marker")); + assert_eq!(parsed.max_uploads, 100); - let (prefix, key_marker, max_uploads) = - parse_list_multipart_uploads_params(None, None, None).expect("expected default params"); - assert_eq!(prefix, ""); - assert_eq!(key_marker, None); - assert_eq!(max_uploads, MAX_PARTS_COUNT); + let parsed = parse_list_multipart_uploads_params(None, None, None).expect("expected default params"); + assert_eq!(parsed.prefix, ""); + assert_eq!(parsed.key_marker, None); + assert_eq!(parsed.max_uploads, MAX_PARTS_COUNT); } #[test]