From d7610eff410fae2ee2d715fc281e0c36cb92886a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 30 Jan 2023 11:52:30 +0000 Subject: [PATCH] refactor(http): [#159] move mods to folders We will use one mod per type of request and response. --- tests/http/asserts.rs | 9 +-- tests/http/client.rs | 8 +- .../{requests.rs => requests/announce.rs} | 30 +++---- tests/http/requests/mod.rs | 1 + .../{responses.rs => responses/announce.rs} | 8 +- tests/http/responses/mod.rs | 1 + tests/http_tracker.rs | 79 ++++++++++--------- 7 files changed, 69 insertions(+), 67 deletions(-) rename tests/http/{requests.rs => requests/announce.rs} (93%) create mode 100644 tests/http/requests/mod.rs rename tests/http/{responses.rs => responses/announce.rs} (94%) create mode 100644 tests/http/responses/mod.rs diff --git a/tests/http/asserts.rs b/tests/http/asserts.rs index cf4683a7b..ec31b1ee4 100644 --- a/tests/http/asserts.rs +++ b/tests/http/asserts.rs @@ -1,7 +1,6 @@ use reqwest::Response; -use super::responses::{Announce, DecodedCompactAnnounce}; -use crate::http::responses::{CompactAnnounce, Error}; +use super::responses::announce::{Announce, Compact, DecodedCompact, Error}; pub async fn assert_empty_announce_response(response: Response) { assert_eq!(response.status(), 200); @@ -22,18 +21,18 @@ pub async fn assert_announce_response(response: Response, expected_announce_resp /// ```text /// b"d8:intervali120e12:min intervali120e8:completei2e10:incompletei0e5:peers6:~\0\0\x01\x1f\x90e6:peers60:e" /// ``` -pub async fn assert_compact_announce_response(response: Response, expected_response: &DecodedCompactAnnounce) { +pub async fn assert_compact_announce_response(response: Response, expected_response: &DecodedCompact) { assert_eq!(response.status(), 200); let bytes = response.bytes().await.unwrap(); - let compact_announce: CompactAnnounce = serde_bencode::from_bytes(&bytes).unwrap_or_else(|_| { + let compact_announce: Compact = serde_bencode::from_bytes(&bytes).unwrap_or_else(|_| { panic!( "response body should be a valid compact announce response, got \"{:?}\"", &bytes ) }); - let actual_response = DecodedCompactAnnounce::from(compact_announce); + let actual_response = DecodedCompact::from(compact_announce); assert_eq!(actual_response, *expected_response); } diff --git a/tests/http/client.rs b/tests/http/client.rs index 062484e83..2d53463dd 100644 --- a/tests/http/client.rs +++ b/tests/http/client.rs @@ -4,7 +4,7 @@ use reqwest::{Client as ReqwestClient, Response}; use torrust_tracker::tracker::auth::KeyId; use super::connection_info::ConnectionInfo; -use super::requests::AnnounceQuery; +use super::requests::announce::Query; /// HTTP Tracker Client pub struct Client { @@ -47,11 +47,11 @@ impl Client { } } - pub async fn announce(&self, query: &AnnounceQuery) -> Response { + pub async fn announce(&self, query: &Query) -> Response { self.get(&self.build_announce_path_and_query(query)).await } - pub async fn announce_with_header(&self, query: &AnnounceQuery, key_id: &str, value: &str) -> Response { + pub async fn announce_with_header(&self, query: &Query, key_id: &str, value: &str) -> Response { self.get_with_header(&self.build_announce_path_and_query(query), key_id, value) .await } @@ -69,7 +69,7 @@ impl Client { .unwrap() } - fn build_announce_path_and_query(&self, query: &AnnounceQuery) -> String { + fn build_announce_path_and_query(&self, query: &Query) -> String { format!("{}?{query}", self.build_path("announce")) } diff --git a/tests/http/requests.rs b/tests/http/requests/announce.rs similarity index 93% rename from tests/http/requests.rs rename to tests/http/requests/announce.rs index 9135020e9..8fe43348f 100644 --- a/tests/http/requests.rs +++ b/tests/http/requests/announce.rs @@ -7,7 +7,7 @@ use serde_repr::Serialize_repr; use torrust_tracker::protocol::info_hash::InfoHash; use torrust_tracker::tracker::peer::Id; -pub struct AnnounceQuery { +pub struct Query { pub info_hash: ByteArray20, pub peer_addr: IpAddr, pub downloaded: BaseTenASCII, @@ -19,7 +19,7 @@ pub struct AnnounceQuery { pub compact: Option, } -impl fmt::Display for AnnounceQuery { +impl fmt::Display for Query { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.build()) } @@ -30,7 +30,7 @@ impl fmt::Display for AnnounceQuery { /// /// /// Some parameters in the specification are not implemented in this tracker yet. -impl AnnounceQuery { +impl Query { /// It builds the URL query component for the announce request. /// /// This custom URL query params encoding is needed because `reqwest` does not allow @@ -41,8 +41,8 @@ impl AnnounceQuery { self.params().to_string() } - pub fn params(&self) -> AnnounceQueryParams { - AnnounceQueryParams::from(self) + pub fn params(&self) -> QueryParams { + QueryParams::from(self) } } @@ -82,13 +82,13 @@ impl fmt::Display for Compact { } } -pub struct AnnounceQueryBuilder { - announce_query: AnnounceQuery, +pub struct QueryBuilder { + announce_query: Query, } -impl AnnounceQueryBuilder { - pub fn default() -> AnnounceQueryBuilder { - let default_announce_query = AnnounceQuery { +impl QueryBuilder { + pub fn default() -> QueryBuilder { + let default_announce_query = Query { info_hash: InfoHash::from_str("9c38422213e30bff212b30c360d26f9a02136422").unwrap().0, peer_addr: IpAddr::V4(Ipv4Addr::new(192, 168, 1, 88)), downloaded: 0, @@ -129,7 +129,7 @@ impl AnnounceQueryBuilder { self } - pub fn query(self) -> AnnounceQuery { + pub fn query(self) -> Query { self.announce_query } } @@ -150,7 +150,7 @@ impl AnnounceQueryBuilder { /// event=completed /// compact=0 /// ``` -pub struct AnnounceQueryParams { +pub struct QueryParams { pub info_hash: Option, pub peer_addr: Option, pub downloaded: Option, @@ -162,7 +162,7 @@ pub struct AnnounceQueryParams { pub compact: Option, } -impl std::fmt::Display for AnnounceQueryParams { +impl std::fmt::Display for QueryParams { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let mut params = vec![]; @@ -204,8 +204,8 @@ impl std::fmt::Display for AnnounceQueryParams { } } -impl AnnounceQueryParams { - pub fn from(announce_query: &AnnounceQuery) -> Self { +impl QueryParams { + pub fn from(announce_query: &Query) -> Self { let event = announce_query.event.as_ref().map(std::string::ToString::to_string); let compact = announce_query.compact.as_ref().map(std::string::ToString::to_string); diff --git a/tests/http/requests/mod.rs b/tests/http/requests/mod.rs new file mode 100644 index 000000000..74894de33 --- /dev/null +++ b/tests/http/requests/mod.rs @@ -0,0 +1 @@ +pub mod announce; diff --git a/tests/http/responses.rs b/tests/http/responses/announce.rs similarity index 94% rename from tests/http/responses.rs rename to tests/http/responses/announce.rs index 7cf283916..6bdc82cdd 100644 --- a/tests/http/responses.rs +++ b/tests/http/responses/announce.rs @@ -31,7 +31,7 @@ impl From for DictionaryPeer { } #[derive(Serialize, Deserialize, Debug, PartialEq)] -pub struct CompactAnnounce { +pub struct Compact { pub complete: u32, pub incomplete: u32, pub interval: u32, @@ -42,7 +42,7 @@ pub struct CompactAnnounce { } #[derive(Debug, PartialEq)] -pub struct DecodedCompactAnnounce { +pub struct DecodedCompact { // code-review: there could be a way to deserialize this struct directly // by using serde instead of doing it manually. Or at least using a custom deserializer. pub complete: u32, @@ -88,8 +88,8 @@ impl CompactPeer { } } -impl From for DecodedCompactAnnounce { - fn from(compact_announce: CompactAnnounce) -> Self { +impl From for DecodedCompact { + fn from(compact_announce: Compact) -> Self { let mut peers = vec![]; for peer_bytes in compact_announce.peers.chunks_exact(6) { diff --git a/tests/http/responses/mod.rs b/tests/http/responses/mod.rs new file mode 100644 index 000000000..74894de33 --- /dev/null +++ b/tests/http/responses/mod.rs @@ -0,0 +1 @@ +pub mod announce; diff --git a/tests/http_tracker.rs b/tests/http_tracker.rs index 91c48c09c..b315f82c2 100644 --- a/tests/http_tracker.rs +++ b/tests/http_tracker.rs @@ -37,8 +37,9 @@ mod http_tracker_server { assert_invalid_peer_id_error_response, assert_is_announce_response, }; use crate::http::client::Client; - use crate::http::requests::{AnnounceQueryBuilder, Compact}; - use crate::http::responses::{self, Announce, CompactAnnounce, CompactPeer, CompactPeerList, DictionaryPeer}; + use crate::http::requests::announce::{Compact, QueryBuilder}; + use crate::http::responses; + use crate::http::responses::announce::{Announce, CompactPeer, CompactPeerList, DictionaryPeer}; use crate::http::server::{ start_default_http_tracker, start_http_tracker_on_reverse_proxy, start_http_tracker_with_external_ip, start_ipv6_http_tracker, start_public_http_tracker, @@ -48,7 +49,7 @@ mod http_tracker_server { async fn should_respond_when_only_the_mandatory_fields_are_provided() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); params.remove_optional_params(); @@ -74,7 +75,7 @@ mod http_tracker_server { // Without `info_hash` param - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); params.info_hash = None; @@ -86,7 +87,7 @@ mod http_tracker_server { // Without `peer_id` param - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); params.peer_id = None; @@ -98,7 +99,7 @@ mod http_tracker_server { // Without `port` param - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); params.port = None; @@ -113,7 +114,7 @@ mod http_tracker_server { async fn should_fail_when_the_info_hash_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); for invalid_value in &invalid_info_hashes() { params.set("info_hash", invalid_value); @@ -135,7 +136,7 @@ mod http_tracker_server { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); params.peer_addr = Some("INVALID-IP-ADDRESS".to_string()); @@ -150,7 +151,7 @@ mod http_tracker_server { async fn should_fail_when_the_downloaded_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = ["-1", "1.1", "a"]; @@ -169,7 +170,7 @@ mod http_tracker_server { async fn should_fail_when_the_uploaded_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = ["-1", "1.1", "a"]; @@ -188,7 +189,7 @@ mod http_tracker_server { async fn should_fail_when_the_peer_id_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = [ "0", @@ -214,7 +215,7 @@ mod http_tracker_server { async fn should_fail_when_the_port_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = ["-1", "1.1", "a"]; @@ -233,7 +234,7 @@ mod http_tracker_server { async fn should_fail_when_the_left_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = ["-1", "1.1", "a"]; @@ -254,7 +255,7 @@ mod http_tracker_server { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = [ "0", @@ -281,7 +282,7 @@ mod http_tracker_server { async fn should_not_fail_when_the_compact_param_is_invalid() { let http_tracker_server = start_default_http_tracker().await; - let mut params = AnnounceQueryBuilder::default().query().params(); + let mut params = QueryBuilder::default().query().params(); let invalid_values = ["-1", "1.1", "a"]; @@ -302,7 +303,7 @@ mod http_tracker_server { let response = Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_info_hash(&InfoHash::from_str("9c38422213e30bff212b30c360d26f9a02136422").unwrap()) .query(), ) @@ -338,7 +339,7 @@ mod http_tracker_server { // Announce the new Peer 2. This new peer is non included on the response peer list let response = Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_id(&peer::Id(*b"-qB00000000000000002")) .query(), @@ -369,7 +370,7 @@ mod http_tracker_server { // Add a peer http_tracker_server.add_torrent(&info_hash, &peer).await; - let announce_query = AnnounceQueryBuilder::default() + let announce_query = QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_id(&peer.peer_id) .query(); @@ -403,7 +404,7 @@ mod http_tracker_server { // Announce the new Peer 2 accepting compact responses let response = Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_id(&peer::Id(*b"-qB00000000000000002")) .with_compact(Compact::Accepted) @@ -411,7 +412,7 @@ mod http_tracker_server { ) .await; - let expected_response = responses::DecodedCompactAnnounce { + let expected_response = responses::announce::DecodedCompact { complete: 2, incomplete: 0, interval: 120, @@ -444,7 +445,7 @@ mod http_tracker_server { // https://www.bittorrent.org/beps/bep_0023.html let response = Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_id(&peer::Id(*b"-qB00000000000000002")) .without_compact() @@ -457,7 +458,7 @@ mod http_tracker_server { async fn is_a_compact_announce_response(response: Response) -> bool { let bytes = response.bytes().await.unwrap(); - let compact_announce = serde_bencode::from_bytes::(&bytes); + let compact_announce = serde_bencode::from_bytes::(&bytes); compact_announce.is_ok() } @@ -466,7 +467,7 @@ mod http_tracker_server { let http_tracker_server = start_public_http_tracker().await; Client::new(http_tracker_server.get_connection_info()) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; let stats = http_tracker_server.tracker.get_stats().await; @@ -479,7 +480,7 @@ mod http_tracker_server { let http_tracker_server = start_ipv6_http_tracker().await; Client::bind(http_tracker_server.get_connection_info(), IpAddr::from_str("::1").unwrap()) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; let stats = http_tracker_server.tracker.get_stats().await; @@ -495,7 +496,7 @@ mod http_tracker_server { Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_peer_addr(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))) .query(), ) @@ -511,7 +512,7 @@ mod http_tracker_server { let http_tracker_server = start_public_http_tracker().await; Client::new(http_tracker_server.get_connection_info()) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; let stats = http_tracker_server.tracker.get_stats().await; @@ -524,7 +525,7 @@ mod http_tracker_server { let http_tracker_server = start_ipv6_http_tracker().await; Client::bind(http_tracker_server.get_connection_info(), IpAddr::from_str("::1").unwrap()) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; let stats = http_tracker_server.tracker.get_stats().await; @@ -540,7 +541,7 @@ mod http_tracker_server { Client::new(http_tracker_server.get_connection_info()) .announce( - &AnnounceQueryBuilder::default() + &QueryBuilder::default() .with_peer_addr(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))) .query(), ) @@ -560,7 +561,7 @@ mod http_tracker_server { let client = Client::bind(http_tracker_server.get_connection_info(), client_ip); - let announce_query = AnnounceQueryBuilder::default() + let announce_query = QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_addr(&IpAddr::from_str("2.2.2.2").unwrap()) .query(); @@ -591,7 +592,7 @@ mod http_tracker_server { let client = Client::bind(http_tracker_server.get_connection_info(), client_ip); - let announce_query = AnnounceQueryBuilder::default() + let announce_query = QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_addr(&IpAddr::from_str("2.2.2.2").unwrap()) .query(); @@ -624,7 +625,7 @@ mod http_tracker_server { let client = Client::bind(http_tracker_server.get_connection_info(), client_ip); - let announce_query = AnnounceQueryBuilder::default() + let announce_query = QueryBuilder::default() .with_info_hash(&info_hash) .with_peer_addr(&IpAddr::from_str("2.2.2.2").unwrap()) .query(); @@ -653,7 +654,7 @@ mod http_tracker_server { let client = Client::new(http_tracker_server.get_connection_info()); - let announce_query = AnnounceQueryBuilder::default().with_info_hash(&info_hash).query(); + let announce_query = QueryBuilder::default().with_info_hash(&info_hash).query(); // todo: shouldn't be the the leftmost IP address? // THe application is taken the the rightmost IP address. See function http::filters::peer_addr @@ -706,7 +707,7 @@ mod http_tracker_server { use crate::http::asserts::{assert_is_announce_response, assert_torrent_not_in_whitelist_error_response}; use crate::http::client::Client; - use crate::http::requests::AnnounceQueryBuilder; + use crate::http::requests::announce::QueryBuilder; use crate::http::server::start_whitelisted_http_tracker; #[tokio::test] @@ -716,7 +717,7 @@ mod http_tracker_server { let info_hash = InfoHash::from_str("9c38422213e30bff212b30c360d26f9a02136422").unwrap(); let response = Client::new(http_tracker_server.get_connection_info()) - .announce(&AnnounceQueryBuilder::default().with_info_hash(&info_hash).query()) + .announce(&QueryBuilder::default().with_info_hash(&info_hash).query()) .await; assert_torrent_not_in_whitelist_error_response(response).await; @@ -735,7 +736,7 @@ mod http_tracker_server { .expect("should add the torrent to the whitelist"); let response = Client::new(http_tracker_server.get_connection_info()) - .announce(&AnnounceQueryBuilder::default().with_info_hash(&info_hash).query()) + .announce(&QueryBuilder::default().with_info_hash(&info_hash).query()) .await; assert_is_announce_response(response).await; @@ -759,7 +760,7 @@ mod http_tracker_server { assert_peer_not_authenticated_error_response, }; use crate::http::client::Client; - use crate::http::requests::AnnounceQueryBuilder; + use crate::http::requests::announce::QueryBuilder; use crate::http::server::start_private_http_tracker; #[tokio::test] @@ -773,7 +774,7 @@ mod http_tracker_server { .unwrap(); let response = Client::authenticated(http_tracker_server.get_connection_info(), key.id()) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; assert_is_announce_response(response).await; @@ -786,7 +787,7 @@ mod http_tracker_server { let info_hash = InfoHash::from_str("9c38422213e30bff212b30c360d26f9a02136422").unwrap(); let response = Client::new(http_tracker_server.get_connection_info()) - .announce(&AnnounceQueryBuilder::default().with_info_hash(&info_hash).query()) + .announce(&QueryBuilder::default().with_info_hash(&info_hash).query()) .await; assert_peer_not_authenticated_error_response(response).await; @@ -800,7 +801,7 @@ mod http_tracker_server { let unregistered_key_id = KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key_id) - .announce(&AnnounceQueryBuilder::default().query()) + .announce(&QueryBuilder::default().query()) .await; assert_invalid_authentication_key_error_response(response).await;