Skip to content

Commit aa7ffdf

Browse files
committed
refactor: [#1159] API client. Extract Origin type
Instead of using a plain string we now use a Origin type containing hte base URL for the API without path or fragments. ``` scheme://host:port/ ```
1 parent e4b9875 commit aa7ffdf

File tree

11 files changed

+179
-40
lines changed

11 files changed

+179
-40
lines changed

Cargo.lock

+2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/tracker-api-client/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,6 @@ version.workspace = true
1818
hyper = "1"
1919
reqwest = { version = "0", features = ["json"] }
2020
serde = { version = "1", features = ["derive"] }
21+
thiserror = "2"
22+
url = { version = "2", features = ["serde"] }
2123
uuid = { version = "1", features = ["v4"] }
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,154 @@
1+
use std::str::FromStr;
2+
3+
use thiserror::Error;
4+
use url::Url;
5+
16
#[derive(Clone)]
27
pub struct ConnectionInfo {
3-
pub bind_address: String,
8+
pub origin: Origin,
49
pub api_token: Option<String>,
510
}
611

712
impl ConnectionInfo {
813
#[must_use]
9-
pub fn authenticated(bind_address: &str, api_token: &str) -> Self {
14+
pub fn authenticated(origin: Origin, api_token: &str) -> Self {
1015
Self {
11-
bind_address: bind_address.to_string(),
16+
origin,
1217
api_token: Some(api_token.to_string()),
1318
}
1419
}
1520

1621
#[must_use]
17-
pub fn anonymous(bind_address: &str) -> Self {
18-
Self {
19-
bind_address: bind_address.to_string(),
20-
api_token: None,
22+
pub fn anonymous(origin: Origin) -> Self {
23+
Self { origin, api_token: None }
24+
}
25+
}
26+
27+
/// Represents the origin of a HTTP request.
28+
///
29+
/// The format of the origin is a URL, but only the scheme, host, and port are used.
30+
///
31+
/// Pattern: `scheme://host:port/`
32+
#[derive(Debug, Clone)]
33+
pub struct Origin {
34+
url: Url,
35+
}
36+
37+
#[derive(Debug, Error)]
38+
pub enum OriginError {
39+
#[error("Invalid URL: {0}")]
40+
InvalidUrl(#[from] url::ParseError),
41+
42+
#[error("URL is missing scheme or host")]
43+
InvalidOrigin,
44+
45+
#[error("Invalid URL scheme, only http and https are supported")]
46+
InvalidScheme,
47+
}
48+
49+
impl FromStr for Origin {
50+
type Err = OriginError;
51+
52+
fn from_str(s: &str) -> Result<Self, Self::Err> {
53+
let mut url = Url::parse(s).map_err(OriginError::InvalidUrl)?;
54+
55+
// Ensure the URL has a scheme and host
56+
if url.scheme().is_empty() || url.host().is_none() {
57+
return Err(OriginError::InvalidOrigin);
58+
}
59+
60+
if url.scheme() != "http" && url.scheme() != "https" {
61+
return Err(OriginError::InvalidScheme);
62+
}
63+
64+
// Retain only the origin components
65+
url.set_path("/");
66+
url.set_query(None);
67+
url.set_fragment(None);
68+
69+
Ok(Origin { url })
70+
}
71+
}
72+
73+
impl std::fmt::Display for Origin {
74+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
75+
write!(f, "{}", self.url)
76+
}
77+
}
78+
79+
impl Origin {
80+
/// # Errors
81+
///
82+
/// Will return an error if the string is not a valid URL containing a
83+
/// scheme and host.
84+
pub fn new(s: &str) -> Result<Self, OriginError> {
85+
s.parse()
86+
}
87+
88+
#[must_use]
89+
pub fn url(&self) -> &Url {
90+
&self.url
91+
}
92+
}
93+
94+
#[cfg(test)]
95+
mod tests {
96+
mod origin {
97+
use crate::connection_info::Origin;
98+
99+
#[test]
100+
fn should_be_parsed_from_a_string_representing_a_url() {
101+
let origin = Origin::new("https://example.com:8080/path?query#fragment").unwrap();
102+
103+
assert_eq!(origin.to_string(), "https://example.com:8080/");
104+
}
105+
106+
mod when_parsing_from_url_string {
107+
use crate::connection_info::Origin;
108+
109+
#[test]
110+
fn should_ignore_default_ports() {
111+
let origin = Origin::new("http://example.com:80").unwrap(); // DevSkim: ignore DS137138
112+
assert_eq!(origin.to_string(), "http://example.com/"); // DevSkim: ignore DS137138
113+
114+
let origin = Origin::new("https://example.com:443").unwrap();
115+
assert_eq!(origin.to_string(), "https://example.com/");
116+
}
117+
118+
#[test]
119+
fn should_add_the_slash_after_the_host() {
120+
let origin = Origin::new("https://example.com:1212").unwrap();
121+
122+
assert_eq!(origin.to_string(), "https://example.com:1212/");
123+
}
124+
125+
#[test]
126+
fn should_remove_extra_path_and_query_parameters() {
127+
let origin = Origin::new("https://example.com:1212/path/to/resource?query=1#fragment").unwrap();
128+
129+
assert_eq!(origin.to_string(), "https://example.com:1212/");
130+
}
131+
132+
#[test]
133+
fn should_fail_when_the_scheme_is_missing() {
134+
let result = Origin::new("example.com");
135+
136+
assert!(result.is_err());
137+
}
138+
139+
#[test]
140+
fn should_fail_when_the_scheme_is_not_supported() {
141+
let result = Origin::new("udp://example.com");
142+
143+
assert!(result.is_err());
144+
}
145+
146+
#[test]
147+
fn should_fail_when_the_host_is_missing() {
148+
let result = Origin::new("http://");
149+
150+
assert!(result.is_err());
151+
}
21152
}
22153
}
23154
}

packages/tracker-api-client/src/v1/client.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use hyper::HeaderMap;
22
use reqwest::Response;
33
use serde::Serialize;
4+
use url::Url;
45
use uuid::Uuid;
56

67
use crate::common::http::{Query, QueryParam, ReqwestQuery};
@@ -17,7 +18,7 @@ impl Client {
1718
pub fn new(connection_info: ConnectionInfo) -> Self {
1819
Self {
1920
connection_info,
20-
base_path: "/api/v1/".to_string(),
21+
base_path: "api/v1/".to_string(),
2122
}
2223
}
2324

@@ -121,11 +122,11 @@ impl Client {
121122
}
122123

123124
pub async fn get_request_with_query(&self, path: &str, params: Query, headers: Option<HeaderMap>) -> Response {
124-
get(&self.base_url(path), Some(params), headers).await
125+
get(self.base_url(path), Some(params), headers).await
125126
}
126127

127128
pub async fn get_request(&self, path: &str) -> Response {
128-
get(&self.base_url(path), None, None).await
129+
get(self.base_url(path), None, None).await
129130
}
130131

131132
fn query_with_token(&self) -> Query {
@@ -135,15 +136,15 @@ impl Client {
135136
}
136137
}
137138

138-
fn base_url(&self, path: &str) -> String {
139-
format!("http://{}{}{path}", &self.connection_info.bind_address, &self.base_path)
139+
fn base_url(&self, path: &str) -> Url {
140+
Url::parse(&format!("{}{}{path}", &self.connection_info.origin, &self.base_path)).unwrap()
140141
}
141142
}
142143

143144
/// # Panics
144145
///
145146
/// Will panic if the request can't be sent
146-
pub async fn get(path: &str, query: Option<Query>, headers: Option<HeaderMap>) -> Response {
147+
pub async fn get(path: Url, query: Option<Query>, headers: Option<HeaderMap>) -> Response {
147148
let builder = reqwest::Client::builder().build().unwrap();
148149

149150
let builder = match query {

tests/servers/api/connection_info.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use torrust_tracker_api_client::connection_info::ConnectionInfo;
1+
use torrust_tracker_api_client::connection_info::{ConnectionInfo, Origin};
22

3-
pub fn connection_with_invalid_token(bind_address: &str) -> ConnectionInfo {
4-
ConnectionInfo::authenticated(bind_address, "invalid token")
3+
pub fn connection_with_invalid_token(origin: Origin) -> ConnectionInfo {
4+
ConnectionInfo::authenticated(origin, "invalid token")
55
}
66

7-
pub fn connection_with_no_token(bind_address: &str) -> ConnectionInfo {
8-
ConnectionInfo::anonymous(bind_address)
7+
pub fn connection_with_no_token(origin: Origin) -> ConnectionInfo {
8+
ConnectionInfo::anonymous(origin)
99
}

tests/servers/api/environment.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::sync::Arc;
44
use bittorrent_primitives::info_hash::InfoHash;
55
use futures::executor::block_on;
66
use tokio::sync::RwLock;
7-
use torrust_tracker_api_client::connection_info::ConnectionInfo;
7+
use torrust_tracker_api_client::connection_info::{ConnectionInfo, Origin};
88
use torrust_tracker_configuration::{Configuration, HttpApi};
99
use torrust_tracker_lib::bootstrap::app::initialize_with_configuration;
1010
use torrust_tracker_lib::bootstrap::jobs::make_rust_tls;
@@ -92,8 +92,10 @@ impl Environment<Running> {
9292
}
9393

9494
pub fn get_connection_info(&self) -> ConnectionInfo {
95+
let origin = Origin::new(&format!("http://{}/", self.server.state.local_addr)).unwrap(); // DevSkim: ignore DS137138
96+
9597
ConnectionInfo {
96-
bind_address: self.server.state.local_addr.to_string(),
98+
origin,
9799
api_token: self.config.access_tokens.get("admin").cloned(),
98100
}
99101
}

tests/servers/api/v1/contract/context/auth_key.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ async fn should_not_allow_generating_a_new_auth_key_for_unauthenticated_users()
8181

8282
let request_id = Uuid::new_v4();
8383

84-
let response = Client::new(connection_with_invalid_token(env.get_connection_info().bind_address.as_str()))
84+
let response = Client::new(connection_with_invalid_token(env.get_connection_info().origin))
8585
.add_auth_key(
8686
AddKeyForm {
8787
opt_key: None,
@@ -100,7 +100,7 @@ async fn should_not_allow_generating_a_new_auth_key_for_unauthenticated_users()
100100

101101
let request_id = Uuid::new_v4();
102102

103-
let response = Client::new(connection_with_no_token(env.get_connection_info().bind_address.as_str()))
103+
let response = Client::new(connection_with_no_token(env.get_connection_info().origin))
104104
.add_auth_key(
105105
AddKeyForm {
106106
opt_key: None,
@@ -332,7 +332,7 @@ async fn should_not_allow_deleting_an_auth_key_for_unauthenticated_users() {
332332

333333
let request_id = Uuid::new_v4();
334334

335-
let response = Client::new(connection_with_invalid_token(env.get_connection_info().bind_address.as_str()))
335+
let response = Client::new(connection_with_invalid_token(env.get_connection_info().origin))
336336
.delete_auth_key(&auth_key.key.to_string(), Some(headers_with_request_id(request_id)))
337337
.await;
338338

@@ -352,7 +352,7 @@ async fn should_not_allow_deleting_an_auth_key_for_unauthenticated_users() {
352352

353353
let request_id = Uuid::new_v4();
354354

355-
let response = Client::new(connection_with_no_token(env.get_connection_info().bind_address.as_str()))
355+
let response = Client::new(connection_with_no_token(env.get_connection_info().origin))
356356
.delete_auth_key(&auth_key.key.to_string(), Some(headers_with_request_id(request_id)))
357357
.await;
358358

@@ -433,7 +433,7 @@ async fn should_not_allow_reloading_keys_for_unauthenticated_users() {
433433

434434
let request_id = Uuid::new_v4();
435435

436-
let response = Client::new(connection_with_invalid_token(env.get_connection_info().bind_address.as_str()))
436+
let response = Client::new(connection_with_invalid_token(env.get_connection_info().origin))
437437
.reload_keys(Some(headers_with_request_id(request_id)))
438438
.await;
439439

@@ -446,7 +446,7 @@ async fn should_not_allow_reloading_keys_for_unauthenticated_users() {
446446

447447
let request_id = Uuid::new_v4();
448448

449-
let response = Client::new(connection_with_no_token(env.get_connection_info().bind_address.as_str()))
449+
let response = Client::new(connection_with_no_token(env.get_connection_info().origin))
450450
.reload_keys(Some(headers_with_request_id(request_id)))
451451
.await;
452452

@@ -507,13 +507,13 @@ mod deprecated_generate_key_endpoint {
507507
let request_id = Uuid::new_v4();
508508
let seconds_valid = 60;
509509

510-
let response = Client::new(connection_with_invalid_token(env.get_connection_info().bind_address.as_str()))
510+
let response = Client::new(connection_with_invalid_token(env.get_connection_info().origin))
511511
.generate_auth_key(seconds_valid, Some(headers_with_request_id(request_id)))
512512
.await;
513513

514514
assert_token_not_valid(response).await;
515515

516-
let response = Client::new(connection_with_no_token(env.get_connection_info().bind_address.as_str()))
516+
let response = Client::new(connection_with_no_token(env.get_connection_info().origin))
517517
.generate_auth_key(seconds_valid, None)
518518
.await;
519519

tests/servers/api/v1/contract/context/health_check.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use torrust_tracker_api_client::v1::client::get;
22
use torrust_tracker_lib::servers::apis::v1::context::health_check::resources::{Report, Status};
33
use torrust_tracker_test_helpers::configuration;
4+
use url::Url;
45

56
use crate::common::logging;
67
use crate::servers::api::Started;
@@ -11,9 +12,9 @@ async fn health_check_endpoint_should_return_status_ok_if_api_is_running() {
1112

1213
let env = Started::new(&configuration::ephemeral().into()).await;
1314

14-
let url = format!("http://{}/api/health_check", env.get_connection_info().bind_address);
15+
let url = Url::parse(&format!("{}api/health_check", env.get_connection_info().origin)).unwrap();
1516

16-
let response = get(&url, None, None).await;
17+
let response = get(url, None, None).await;
1718

1819
assert_eq!(response.status(), 200);
1920
assert_eq!(response.headers().get("content-type").unwrap(), "application/json");

tests/servers/api/v1/contract/context/stats.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ async fn should_not_allow_getting_tracker_statistics_for_unauthenticated_users()
7979

8080
let request_id = Uuid::new_v4();
8181

82-
let response = Client::new(connection_with_invalid_token(env.get_connection_info().bind_address.as_str()))
82+
let response = Client::new(connection_with_invalid_token(env.get_connection_info().origin))
8383
.get_tracker_statistics(Some(headers_with_request_id(request_id)))
8484
.await;
8585

@@ -92,7 +92,7 @@ async fn should_not_allow_getting_tracker_statistics_for_unauthenticated_users()
9292

9393
let request_id = Uuid::new_v4();
9494

95-
let response = Client::new(connection_with_no_token(env.get_connection_info().bind_address.as_str()))
95+
let response = Client::new(connection_with_no_token(env.get_connection_info().origin))
9696
.get_tracker_statistics(Some(headers_with_request_id(request_id)))
9797
.await;
9898

0 commit comments

Comments
 (0)