From 58735e62bf429daa5ebd5f76382e1a0683f774a1 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Dec 2024 15:41:48 +0100 Subject: [PATCH 1/3] tests/crates/list: Simplify `multiple_ids()` query parameters --- src/tests/routes/crates/list.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index fb9cf3ef3c3..bb146518525 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -664,12 +664,8 @@ async fn multiple_ids() -> anyhow::Result<()> { .expect_build(&mut conn) .await; - for json in search_both( - &anon, - "ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown", - ) - .await - { + let query = "ids[]=foo&ids[]=bar&ids[]=baz&ids[]=baz&ids[]=unknown"; + for json in search_both(&anon, query).await { assert_eq!(json.meta.total, 3); assert_eq!(json.crates[0].name, "bar"); assert_eq!(json.crates[1].name, "baz"); From 0044c4200eedd380bd546a5e6d4e1860afaf3288 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Dec 2024 15:46:03 +0100 Subject: [PATCH 2/3] tests/crates/list: Add snapshot assertions for `next_page` serialization --- src/tests/routes/crates/list.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index bb146518525..fecc38fb5e4 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -672,6 +672,13 @@ async fn multiple_ids() -> anyhow::Result<()> { assert_eq!(json.crates[2].name, "foo"); } + let response = anon.search(&format!("{query}&per_page=1&page=2")).await; + assert_snapshot!(response.meta.prev_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&page=1"); + assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&page=3"); + + let response = anon.search(&format!("{query}&per_page=1")).await; + assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&seek=Mg"); + Ok(()) } From 2764c4394ea715d8e7969cec64290cbe97e08310 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Thu, 12 Dec 2024 16:12:55 +0100 Subject: [PATCH 3/3] util/request_helper: Fix `query_with_params()` behavior for duplicate keys Our search endpoint supports having multiple `ids[]` query parameters defined, but the way we generate our paginations fields only considered the last such query parameter until now. This commit fixes the code to include all query parameters instead. --- src/tests/routes/crates/list.rs | 14 +++++++------- src/util/request_helpers.rs | 9 ++++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index fecc38fb5e4..9e586f7e3f7 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -673,11 +673,11 @@ async fn multiple_ids() -> anyhow::Result<()> { } let response = anon.search(&format!("{query}&per_page=1&page=2")).await; - assert_snapshot!(response.meta.prev_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&page=1"); - assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&page=3"); + assert_snapshot!(response.meta.prev_page.unwrap(), @"?ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown&per_page=1&page=1"); + assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown&per_page=1&page=3"); let response = anon.search(&format!("{query}&per_page=1")).await; - assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=unknown&per_page=1&seek=Mg"); + assert_snapshot!(response.meta.next_page.unwrap(), @"?ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown&per_page=1&seek=Mg"); Ok(()) } @@ -1016,21 +1016,21 @@ async fn pagination_links_included_if_applicable() -> anyhow::Result<()> { let page4 = anon.search("letter=p&page=4&per_page=1").await; assert_eq!( - Some("?letter=p&page=2&per_page=1".to_string()), + Some("?letter=p&per_page=1&page=2".to_string()), page1.meta.next_page ); assert_eq!(None, page1.meta.prev_page); assert_eq!( - Some("?letter=p&page=3&per_page=1".to_string()), + Some("?letter=p&per_page=1&page=3".to_string()), page2.meta.next_page ); assert_eq!( - Some("?letter=p&page=1&per_page=1".to_string()), + Some("?letter=p&per_page=1&page=1".to_string()), page2.meta.prev_page ); assert_eq!(None, page4.meta.next_page); assert_eq!( - Some("?letter=p&page=2&per_page=1".to_string()), + Some("?letter=p&per_page=1&page=2".to_string()), page3.meta.prev_page ); assert!([page1.meta.total, page2.meta.total, page3.meta.total] diff --git a/src/util/request_helpers.rs b/src/util/request_helpers.rs index bd17724073f..0fd9f383760 100644 --- a/src/util/request_helpers.rs +++ b/src/util/request_helpers.rs @@ -47,8 +47,15 @@ impl RequestUtils for T { } fn query_with_params(&self, new_params: IndexMap) -> String { - let mut params = self.query(); + let query_bytes = self.uri().query().unwrap_or("").as_bytes(); + + let mut params = url::form_urlencoded::parse(query_bytes) + .into_owned() + .filter(|(key, _)| !new_params.contains_key(key)) + .collect::>(); + params.extend(new_params); + let query_string = url::form_urlencoded::Serializer::new(String::new()) .extend_pairs(params) .finish();