Merge pull request 'Allow multi-character delimiters in List* (fix #692)' (#711) from multi-char-delimiter-692 into main

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/711

3 files changed, 67 insertions, 7 deletions

diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs
index 4b815f79..0065ca59 100644
--- a/src/api/s3/api_server.rs
+++ b/src/api/s3/api_server.rs
@@ -260,7 +260,7 @@ impl ApiHandler for S3ApiServer {
 						common: ListQueryCommon {
 							bucket_name,
 							bucket_id,
-							delimiter: delimiter.map(|d| d.to_string()),
+							delimiter,
 							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),
@@ -290,7 +290,7 @@ impl ApiHandler for S3ApiServer {
 							common: ListQueryCommon {
 								bucket_name,
 								bucket_id,
-								delimiter: delimiter.map(|d| d.to_string()),
+								delimiter,
 								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(),
@@ -323,7 +323,7 @@ impl ApiHandler for S3ApiServer {
 						common: ListQueryCommon {
 							bucket_name,
 							bucket_id,
-							delimiter: delimiter.map(|d| d.to_string()),
+							delimiter,
 							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),
diff --git a/src/api/s3/router.rs b/src/api/s3/router.rs
index 821b0e07..05882885 100644
--- a/src/api/s3/router.rs
+++ b/src/api/s3/router.rs
@@ -170,7 +170,7 @@ pub enum Endpoint {
 	},
 	ListBuckets,
 	ListMultipartUploads {
-		delimiter: Option<char>,
+		delimiter: Option<String>,
 		encoding_type: Option<String>,
 		key_marker: Option<String>,
 		max_uploads: Option<usize>,
@@ -178,7 +178,7 @@ pub enum Endpoint {
 		upload_id_marker: Option<String>,
 	},
 	ListObjects {
-		delimiter: Option<char>,
+		delimiter: Option<String>,
 		encoding_type: Option<String>,
 		marker: Option<String>,
 		max_keys: Option<usize>,
@@ -188,7 +188,7 @@ pub enum Endpoint {
 		// This value should always be 2. It is not checked when constructing the struct
 		list_type: String,
 		continuation_token: Option<String>,
-		delimiter: Option<char>,
+		delimiter: Option<String>,
 		encoding_type: Option<String>,
 		fetch_owner: Option<bool>,
 		max_keys: Option<usize>,
@@ -196,7 +196,7 @@ pub enum Endpoint {
 		start_after: Option<String>,
 	},
 	ListObjectVersions {
-		delimiter: Option<char>,
+		delimiter: Option<String>,
 		encoding_type: Option<String>,
 		key_marker: Option<String>,
 		max_keys: Option<u64>,
diff --git a/src/garage/tests/s3/list.rs b/src/garage/tests/s3/list.rs
index bb03f250..1b0c006d 100644
--- a/src/garage/tests/s3/list.rs
+++ b/src/garage/tests/s3/list.rs
@@ -613,3 +613,63 @@ async fn test_listmultipart() {
 		assert!(r.common_prefixes.is_none());
 	}
 }
+
+#[tokio::test]
+async fn test_multichar_delimiter() {
+	// Test case from dpape from issue #692 with reference results from Amazon
+
+	let ctx = common::context();
+	let bucket = ctx.create_bucket("multichardelim");
+
+	for k in [
+		"a/", "a/b/", "a/b/c/", "a/b/c/d", "a/c/", "a/c/b/", "a/c/b/e",
+	] {
+		ctx.client
+			.put_object()
+			.bucket(&bucket)
+			.key(k)
+			.send()
+			.await
+			.unwrap();
+	}
+
+	// With delimiter /
+	{
+		let r = ctx
+			.client
+			.list_objects_v2()
+			.bucket(&bucket)
+			.delimiter("/")
+			.send()
+			.await
+			.unwrap();
+
+		assert!(r.contents.is_none());
+
+		let common_prefixes = r.common_prefixes.unwrap();
+		assert_eq!(common_prefixes.len(), 1);
+		assert_eq!(common_prefixes[0].prefix.as_deref().unwrap(), "a/");
+	}
+
+	// With delimiter b/
+	{
+		let r = ctx
+			.client
+			.list_objects_v2()
+			.bucket(&bucket)
+			.delimiter("b/")
+			.send()
+			.await
+			.unwrap();
+
+		let contents = r.contents.unwrap();
+		assert_eq!(contents.len(), 2);
+		assert_eq!(contents[0].key.as_deref().unwrap(), "a/");
+		assert_eq!(contents[1].key.as_deref().unwrap(), "a/c/");
+
+		let common_prefixes = r.common_prefixes.unwrap();
+		assert_eq!(common_prefixes.len(), 2);
+		assert_eq!(common_prefixes[0].prefix.as_deref().unwrap(), "a/b/");
+		assert_eq!(common_prefixes[1].prefix.as_deref().unwrap(), "a/c/b/");
+	}
+}