fix test; simplify code

src/api/s3/api_server.rs

@@ -257,7 +257,7 @@ impl ApiHandler for S3ApiServer {
 							bucket_name,
 							bucket_id,
 							delimiter: delimiter.map(|d| d.to_string()),
-							page_size: max_keys.map(|p| p.clamp(1, 1000)).unwrap_or(1000),
+							page_size: max_keys.unwrap_or(1000).clamp(1, 1000),
 							prefix: prefix.unwrap_or_default(),
 							urlencode_resp: encoding_type.map(|e| e == "url").unwrap_or(false),
 						},
@@ -287,7 +287,7 @@ impl ApiHandler for S3ApiServer {
 								bucket_name,
 								bucket_id,
 								delimiter: delimiter.map(|d| d.to_string()),
-								page_size: max_keys.map(|p| p.clamp(1, 1000)).unwrap_or(1000),
+								page_size: max_keys.unwrap_or(1000).clamp(1, 1000),
 								urlencode_resp: encoding_type.map(|e| e == "url").unwrap_or(false),
 								prefix: prefix.unwrap_or_default(),
 							},
@@ -320,7 +320,7 @@ impl ApiHandler for S3ApiServer {
 							bucket_name,
 							bucket_id,
 							delimiter: delimiter.map(|d| d.to_string()),
-							page_size: max_uploads.map(|p| p.clamp(1, 1000)).unwrap_or(1000),
+							page_size: max_uploads.unwrap_or(1000).clamp(1, 1000),
 							prefix: prefix.unwrap_or_default(),
 							urlencode_resp: encoding_type.map(|e| e == "url").unwrap_or(false),
 						},
@@ -344,7 +344,7 @@ impl ApiHandler for S3ApiServer {
 						key,
 						upload_id,
 						part_number_marker: part_number_marker.map(|p| p.clamp(1, 10000)),
-						max_parts: max_parts.map(|p| p.clamp(1, 1000)).unwrap_or(1000),
+						max_parts: max_parts.unwrap_or(1000).clamp(1, 1000),
 					},
 				)
 				.await

src/api/s3/list.rs

@@ -282,15 +282,17 @@ pub async fn handle_list_parts(
 
 	let result = s3_xml::ListPartsResult {
 		xmlns: (),
+
 		// Query parameters
 		bucket: s3_xml::Value(query.bucket_name.to_string()),
 		key: s3_xml::Value(query.key.to_string()),
 		upload_id: s3_xml::Value(query.upload_id.to_string()),
 		part_number_marker: query.part_number_marker.map(|e| s3_xml::IntValue(e as i64)),
 		max_parts: s3_xml::IntValue(query.max_parts as i64),
+
 		// Result values
 		next_part_number_marker: next.map(|e| s3_xml::IntValue(e as i64)),
-		is_truncated: s3_xml::Value(next.map(|_| "true").unwrap_or("false").to_string()),
+		is_truncated: s3_xml::Value(format!("{}", next.is_some())),
 		parts: info
 			.iter()
 			.map(|part| s3_xml::PartItem {
@@ -300,6 +302,7 @@ pub async fn handle_list_parts(
 				size: s3_xml::IntValue(part.size as i64),
 			})
 			.collect(),
+
 		// Dummy result values (unsupported features)
 		initiator: s3_xml::Initiator {
 			display_name: s3_xml::Value(DUMMY_NAME.to_string()),
@@ -462,6 +465,8 @@ fn fetch_part_info<'a>(
 	query: &ListPartsQuery,
 	mpu: &'a MultipartUpload,
 ) -> Result<(Vec<PartInfo<'a>>, Option<u64>), Error> {
+	assert!((1..=1000).contains(&query.max_parts)); // see s3/api_server.rs
+
 	// Parse multipart upload part list, removing parts not yet finished
 	// and failed part uploads that were overwritten
 	let mut parts: Vec<PartInfo<'a>> = Vec::with_capacity(mpu.parts.items().len());
@@ -496,10 +501,8 @@ fn fetch_part_info<'a>(
 	// Cut the end if we have too many parts
 	if parts.len() > query.max_parts as usize {
 		parts.truncate(query.max_parts as usize);
-		if let Some(part_info) = parts.last() {
-			let pagination = Some(part_info.part_number);
-			return Ok((parts, pagination));
-		}
+		let pagination = Some(parts.last().unwrap().part_number);
+		return Ok((parts, pagination));
 	}
 
 	Ok((parts, None))

src/garage/tests/s3/multipart.rs

@@ -65,7 +65,8 @@ async fn test_uploadlistpart() {
 
 		let ps = r.parts.unwrap();
 		assert_eq!(ps.len(), 1);
-		let fp = ps.iter().find(|x| x.part_number == 2).unwrap();
+		assert_eq!(ps[0].part_number, 2);
+		let fp = &ps[0];
 		assert!(fp.last_modified.is_some());
 		assert_eq!(
 			fp.e_tag.as_ref().unwrap(),
@@ -100,13 +101,24 @@ async fn test_uploadlistpart() {
 
 		let ps = r.parts.unwrap();
 		assert_eq!(ps.len(), 2);
-		let fp = ps.iter().find(|x| x.part_number == 1).unwrap();
+
+		assert_eq!(ps[0].part_number, 1);
+		let fp = &ps[0];
 		assert!(fp.last_modified.is_some());
 		assert_eq!(
 			fp.e_tag.as_ref().unwrap(),
 			"\"3c484266f9315485694556e6c693bfa2\""
 		);
 		assert_eq!(fp.size, SZ_5MB as i64);
+
+		assert_eq!(ps[1].part_number, 2);
+		let fp = &ps[1];
+		assert!(fp.last_modified.is_some());
+		assert_eq!(
+			fp.e_tag.as_ref().unwrap(),
+			"\"3366bb9dcf710d6801b5926467d02e19\""
+		);
+		assert_eq!(fp.size, SZ_5MB as i64);
 	}
 
 	{
@@ -123,12 +135,19 @@ async fn test_uploadlistpart() {
 			.unwrap();
 
 		assert!(r.part_number_marker.is_none());
-		assert!(r.next_part_number_marker.is_some());
+		assert_eq!(r.next_part_number_marker.as_deref(), Some("1"));
 		assert_eq!(r.max_parts, 1_i32);
 		assert!(r.is_truncated);
 		assert_eq!(r.key.unwrap(), "a");
 		assert_eq!(r.upload_id.unwrap().as_str(), uid.as_str());
-		assert_eq!(r.parts.unwrap().len(), 1);
+		let parts = r.parts.unwrap();
+		assert_eq!(parts.len(), 1);
+		let fp = &parts[0];
+		assert_eq!(fp.part_number, 1);
+		assert_eq!(
+			fp.e_tag.as_ref().unwrap(),
+			"\"3c484266f9315485694556e6c693bfa2\""
+		);
 
 		let r2 = ctx
 			.client
@@ -147,10 +166,18 @@ async fn test_uploadlistpart() {
 			r.next_part_number_marker.as_ref().unwrap()
 		);
 		assert_eq!(r2.max_parts, 1_i32);
-		assert!(r2.is_truncated);
 		assert_eq!(r2.key.unwrap(), "a");
 		assert_eq!(r2.upload_id.unwrap().as_str(), uid.as_str());
-		assert_eq!(r2.parts.unwrap().len(), 1);
+		let parts = r2.parts.unwrap();
+		assert_eq!(parts.len(), 1);
+		let fp = &parts[0];
+		assert_eq!(fp.part_number, 2);
+		assert_eq!(
+			fp.e_tag.as_ref().unwrap(),
+			"\"3366bb9dcf710d6801b5926467d02e19\""
+		);
+		//assert!(r2.is_truncated);   // WHY? (this was the test before)
+		assert!(!r2.is_truncated);
 	}
 
 	let cmp = CompletedMultipartUpload::builder()