Skip to content

Commit e88b1d6

Browse files
committed
feat(http): [#192] Query struct allows multiple values for the same param
The `torrust_tracker::http::axum_implementation::query` allow mutiple values for the same URL query param, for example: ``` param1=value1&param2=value2 ``` It's needed in the `scrape` request: http://localhost:7070/scrape?info_hash=%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0&info_hash=%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0
1 parent 0cab696 commit e88b1d6

File tree

1 file changed

+157
-72
lines changed

1 file changed

+157
-72
lines changed
Lines changed: 157 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,50 @@
1-
use std::collections::HashMap;
21
use std::panic::Location;
32
use std::str::FromStr;
43

4+
use multimap::MultiMap;
55
use thiserror::Error;
66

7-
/// Represent a URL query component with some restrictions.
8-
/// It does not allow duplicate param names like this: `param1=value1&param1=value2`
9-
/// It would take the second value for `param1`.
7+
type ParamName = String;
8+
type ParamValue = String;
9+
10+
/// Represent a URL query component:
11+
///
12+
/// ```text
13+
/// URI = scheme ":" ["//" authority] path ["?" query] ["#" fragment]
14+
/// ```
15+
#[derive(Debug)]
1016
pub struct Query {
1117
/* code-review:
12-
- Consider using `HashMap<String, Param>`, because it does not allow you to add a second value for the same param name.
1318
- Consider using a third-party crate.
1419
- Conversion from/to string is not deterministic. Params can be in a different order in the query string.
1520
*/
16-
params: HashMap<String, String>,
21+
params: MultiMap<ParamName, NameValuePair>,
22+
}
23+
24+
impl Query {
25+
/// Returns only the first param value even if it has multiple values like this:
26+
///
27+
/// ```text
28+
/// param1=value1&param1=value2
29+
/// ```
30+
///
31+
/// In that case `get_param("param1")` will return `value1`.
32+
#[must_use]
33+
pub fn get_param(&self, name: &str) -> Option<String> {
34+
self.params.get(name).map(|pair| pair.value.clone())
35+
}
36+
37+
/// Returns all the param values as a vector even if it has only one value.
38+
#[must_use]
39+
pub fn get_param_vec(&self, name: &str) -> Option<Vec<String>> {
40+
self.params.get_vec(name).map(|pairs| {
41+
let mut param_values = vec![];
42+
for pair in pairs {
43+
param_values.push(pair.value.to_string());
44+
}
45+
param_values
46+
})
47+
}
1748
}
1849

1950
#[derive(Error, Debug)]
@@ -29,13 +60,14 @@ impl FromStr for Query {
2960
type Err = ParseQueryError;
3061

3162
fn from_str(raw_query: &str) -> Result<Self, Self::Err> {
32-
let mut params: HashMap<String, String> = HashMap::new();
63+
let mut params: MultiMap<ParamName, NameValuePair> = MultiMap::new();
3364

3465
let raw_params = raw_query.trim().trim_start_matches('?').split('&').collect::<Vec<&str>>();
3566

3667
for raw_param in raw_params {
37-
let param: Param = raw_param.parse()?;
38-
params.insert(param.name, param.value);
68+
let pair: NameValuePair = raw_param.parse()?;
69+
let param_name = pair.name.clone();
70+
params.insert(param_name, pair);
3971
}
4072

4173
Ok(Self { params })
@@ -44,10 +76,10 @@ impl FromStr for Query {
4476

4577
impl From<Vec<(&str, &str)>> for Query {
4678
fn from(raw_params: Vec<(&str, &str)>) -> Self {
47-
let mut params: HashMap<String, String> = HashMap::new();
79+
let mut params: MultiMap<ParamName, NameValuePair> = MultiMap::new();
4880

4981
for raw_param in raw_params {
50-
params.insert(raw_param.0.to_owned(), raw_param.1.to_owned());
82+
params.insert(raw_param.0.to_owned(), NameValuePair::new(raw_param.0, raw_param.1));
5183
}
5284

5385
Self { params }
@@ -58,29 +90,31 @@ impl std::fmt::Display for Query {
5890
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
5991
let query = self
6092
.params
61-
.iter()
62-
.map(|param| format!("{}", Param::new(param.0, param.1)))
93+
.iter_all()
94+
.map(|param| format!("{}", FieldValuePairSet::from_vec(param.1)))
6395
.collect::<Vec<String>>()
6496
.join("&");
6597

6698
write!(f, "{query}")
6799
}
68100
}
69101

70-
impl Query {
71-
#[must_use]
72-
pub fn get_param(&self, name: &str) -> Option<String> {
73-
self.params.get(name).map(std::string::ToString::to_string)
74-
}
102+
#[derive(Debug, PartialEq, Clone)]
103+
struct NameValuePair {
104+
name: ParamName,
105+
value: ParamValue,
75106
}
76107

77-
#[derive(Debug, PartialEq)]
78-
struct Param {
79-
name: String,
80-
value: String,
108+
impl NameValuePair {
109+
pub fn new(name: &str, value: &str) -> Self {
110+
Self {
111+
name: name.to_owned(),
112+
value: value.to_owned(),
113+
}
114+
}
81115
}
82116

83-
impl FromStr for Param {
117+
impl FromStr for NameValuePair {
84118
type Err = ParseQueryError;
85119

86120
fn from_str(raw_param: &str) -> Result<Self, Self::Err> {
@@ -100,18 +134,39 @@ impl FromStr for Param {
100134
}
101135
}
102136

103-
impl std::fmt::Display for Param {
137+
impl std::fmt::Display for NameValuePair {
104138
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
105139
write!(f, "{}={}", self.name, self.value)
106140
}
107141
}
108142

109-
impl Param {
110-
pub fn new(name: &str, value: &str) -> Self {
111-
Self {
112-
name: name.to_owned(),
113-
value: value.to_owned(),
143+
#[derive(Debug, PartialEq)]
144+
struct FieldValuePairSet {
145+
pairs: Vec<NameValuePair>,
146+
}
147+
148+
impl FieldValuePairSet {
149+
fn from_vec(pair_vec: &Vec<NameValuePair>) -> Self {
150+
let mut pairs: Vec<NameValuePair> = vec![];
151+
152+
for pair in pair_vec {
153+
pairs.push(pair.clone());
114154
}
155+
156+
Self { pairs }
157+
}
158+
}
159+
160+
impl std::fmt::Display for FieldValuePairSet {
161+
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
162+
let query = self
163+
.pairs
164+
.iter()
165+
.map(|pair| format!("{pair}"))
166+
.collect::<Vec<String>>()
167+
.join("&");
168+
169+
write!(f, "{query}")
115170
}
116171
}
117172

@@ -136,6 +191,14 @@ mod tests {
136191
assert_eq!(query.get_param("port").unwrap(), "17548");
137192
}
138193

194+
#[test]
195+
fn should_be_instantiated_from_a_string_pair_vector() {
196+
let query = Query::from(vec![("param1", "value1"), ("param2", "value2")]);
197+
198+
assert_eq!(query.get_param("param1"), Some("value1".to_string()));
199+
assert_eq!(query.get_param("param2"), Some("value2".to_string()));
200+
}
201+
139202
#[test]
140203
fn should_fail_parsing_an_invalid_query_string() {
141204
let invalid_raw_query = "name=value=value";
@@ -151,7 +214,7 @@ mod tests {
151214

152215
let query = raw_query.parse::<Query>().unwrap();
153216

154-
assert_eq!(query.get_param("name").unwrap(), "value");
217+
assert_eq!(query.get_param("name"), Some("value".to_string()));
155218
}
156219

157220
#[test]
@@ -160,61 +223,83 @@ mod tests {
160223

161224
let query = raw_query.parse::<Query>().unwrap();
162225

163-
assert_eq!(query.get_param("name").unwrap(), "value");
164-
}
165-
166-
#[test]
167-
fn should_be_instantiated_from_a_string_pair_vector() {
168-
let query = Query::from(vec![("param1", "value1"), ("param2", "value2")]).to_string();
169-
170-
assert!(query == "param1=value1&param2=value2" || query == "param2=value2&param1=value1");
226+
assert_eq!(query.get_param("name"), Some("value".to_string()));
171227
}
172228

173-
#[test]
174-
fn should_not_allow_more_than_one_value_for_the_same_param() {
175-
let query = Query::from(vec![("param1", "value1"), ("param1", "value2"), ("param1", "value3")]).to_string();
176-
177-
assert_eq!(query, "param1=value3");
229+
mod should_allow_more_than_one_value_for_the_same_param {
230+
use crate::http::axum_implementation::query::Query;
231+
232+
#[test]
233+
fn instantiated_from_a_vector() {
234+
let query1 = Query::from(vec![("param1", "value1"), ("param1", "value2")]);
235+
assert_eq!(
236+
query1.get_param_vec("param1"),
237+
Some(vec!["value1".to_string(), "value2".to_string()])
238+
);
239+
}
240+
241+
#[test]
242+
fn parsed_from_an_string() {
243+
let query2 = "param1=value1&param1=value2".parse::<Query>().unwrap();
244+
assert_eq!(
245+
query2.get_param_vec("param1"),
246+
Some(vec!["value1".to_string(), "value2".to_string()])
247+
);
248+
}
178249
}
179250

180-
#[test]
181-
fn should_be_displayed() {
182-
let query = "param1=value1&param2=value2".parse::<Query>().unwrap().to_string();
183-
184-
assert!(query == "param1=value1&param2=value2" || query == "param2=value2&param1=value1");
251+
mod should_be_displayed {
252+
use crate::http::axum_implementation::query::Query;
253+
254+
#[test]
255+
fn with_one_param() {
256+
assert_eq!("param1=value1".parse::<Query>().unwrap().to_string(), "param1=value1");
257+
}
258+
259+
#[test]
260+
fn with_multiple_params() {
261+
let query = "param1=value1&param2=value2".parse::<Query>().unwrap().to_string();
262+
assert!(query == "param1=value1&param2=value2" || query == "param2=value2&param1=value1");
263+
}
264+
265+
#[test]
266+
fn with_multiple_values_for_the_same_param() {
267+
let query = "param1=value1&param1=value2".parse::<Query>().unwrap().to_string();
268+
assert!(query == "param1=value1&param1=value2" || query == "param1=value2&param1=value1");
269+
}
185270
}
186-
}
187271

188-
mod url_query_param {
189-
use crate::http::axum_implementation::query::Param;
272+
mod param_name_value_pair {
273+
use crate::http::axum_implementation::query::NameValuePair;
190274

191-
#[test]
192-
fn should_parse_a_single_query_param() {
193-
let raw_param = "name=value";
275+
#[test]
276+
fn should_parse_a_single_query_param() {
277+
let raw_param = "name=value";
194278

195-
let param = raw_param.parse::<Param>().unwrap();
279+
let param = raw_param.parse::<NameValuePair>().unwrap();
196280

197-
assert_eq!(
198-
param,
199-
Param {
200-
name: "name".to_string(),
201-
value: "value".to_string(),
202-
}
203-
);
204-
}
281+
assert_eq!(
282+
param,
283+
NameValuePair {
284+
name: "name".to_string(),
285+
value: "value".to_string(),
286+
}
287+
);
288+
}
205289

206-
#[test]
207-
fn should_fail_parsing_an_invalid_query_param() {
208-
let invalid_raw_param = "name=value=value";
290+
#[test]
291+
fn should_fail_parsing_an_invalid_query_param() {
292+
let invalid_raw_param = "name=value=value";
209293

210-
let query = invalid_raw_param.parse::<Param>();
294+
let query = invalid_raw_param.parse::<NameValuePair>();
211295

212-
assert!(query.is_err());
213-
}
296+
assert!(query.is_err());
297+
}
214298

215-
#[test]
216-
fn should_be_displayed() {
217-
assert_eq!("name=value".parse::<Param>().unwrap().to_string(), "name=value");
299+
#[test]
300+
fn should_be_displayed() {
301+
assert_eq!("name=value".parse::<NameValuePair>().unwrap().to_string(), "name=value");
302+
}
218303
}
219304
}
220305
}

0 commit comments

Comments
 (0)