From b2fc66331a67d59754db700968bfd76457109135 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 7 Feb 2025 10:03:30 +0000 Subject: [PATCH 1/2] test: [#1247] add tests for bittorrent_tracker_core::announce_handler --- packages/tracker-core/src/announce_handler.rs | 174 ++++++++++++++++-- 1 file changed, 160 insertions(+), 14 deletions(-) diff --git a/packages/tracker-core/src/announce_handler.rs b/packages/tracker-core/src/announce_handler.rs index aa311fe46..5dd4a0291 100644 --- a/packages/tracker-core/src/announce_handler.rs +++ b/packages/tracker-core/src/announce_handler.rs @@ -119,12 +119,7 @@ pub enum PeersWanted { impl PeersWanted { #[must_use] pub fn only(limit: u32) -> Self { - let amount: usize = match limit.try_into() { - Ok(amount) => amount, - Err(_) => TORRENT_PEERS_LIMIT, - }; - - Self::Only { amount } + limit.into() } fn limit(&self) -> usize { @@ -137,13 +132,29 @@ impl PeersWanted { impl From for PeersWanted { fn from(value: i32) -> Self { - if value > 0 { - match value.try_into() { - Ok(peers_wanted) => Self::Only { amount: peers_wanted }, - Err(_) => Self::All, - } - } else { - Self::All + if value <= 0 { + return PeersWanted::All; + } + + // This conversion is safe because `value > 0` + let amount = usize::try_from(value).unwrap(); + + PeersWanted::Only { + amount: amount.min(TORRENT_PEERS_LIMIT), + } + } +} + +impl From for PeersWanted { + fn from(value: u32) -> Self { + if value == 0 { + return PeersWanted::All; + } + + let amount = value as usize; + + PeersWanted::Only { + amount: amount.min(TORRENT_PEERS_LIMIT), } } } @@ -210,6 +221,19 @@ mod tests { } } + /// Sample peer when for tests that need more than two peer + fn sample_peer_3() -> Peer { + Peer { + peer_id: PeerId(*b"-qB00000000000000003"), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 3)), 8082), + updated: DurationSinceUnixEpoch::new(1_669_397_478_934, 0), + uploaded: NumberOfBytes::new(0), + downloaded: NumberOfBytes::new(0), + left: NumberOfBytes::new(0), + event: AnnounceEvent::Completed, + } + } + mod for_all_tracker_config_modes { mod handling_an_announce_request { @@ -217,7 +241,7 @@ mod tests { use std::sync::Arc; use crate::announce_handler::tests::the_announce_handler::{ - peer_ip, public_tracker, sample_peer_1, sample_peer_2, + peer_ip, public_tracker, sample_peer_1, sample_peer_2, sample_peer_3, }; use crate::announce_handler::PeersWanted; use crate::core_tests::{sample_info_hash, sample_peer}; @@ -349,6 +373,38 @@ mod tests { assert_eq!(announce_data.peers, vec![Arc::new(previously_announced_peer)]); } + #[tokio::test] + async fn it_should_allow_peers_to_get_only_a_subset_of_the_peers_in_the_swarm() { + let (announce_handler, _scrape_handler) = public_tracker(); + + let mut previously_announced_peer_1 = sample_peer_1(); + announce_handler.announce( + &sample_info_hash(), + &mut previously_announced_peer_1, + &peer_ip(), + &PeersWanted::All, + ); + + let mut previously_announced_peer_2 = sample_peer_2(); + announce_handler.announce( + &sample_info_hash(), + &mut previously_announced_peer_2, + &peer_ip(), + &PeersWanted::All, + ); + + let mut peer = sample_peer_3(); + let announce_data = + announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::only(1)); + + // It should return only one peer. There is no guarantee on + // which peer will be returned. + assert!( + announce_data.peers == vec![Arc::new(previously_announced_peer_1)] + || announce_data.peers == vec![Arc::new(previously_announced_peer_2)] + ); + } + mod it_should_update_the_swarm_stats_for_the_torrent { use crate::announce_handler::tests::the_announce_handler::{peer_ip, public_tracker}; @@ -461,5 +517,95 @@ mod tests { assert!(torrent_entry.peers_is_empty()); } } + + mod should_allow_the_client_peers_to_specified_the_number_of_peers_wanted { + + use torrust_tracker_configuration::TORRENT_PEERS_LIMIT; + + use crate::announce_handler::PeersWanted; + + #[test] + fn it_should_return_the_maximin_number_of_peers_by_default() { + let peers_wanted = PeersWanted::default(); + + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + } + + #[test] + fn it_should_return_74_at_the_most_if_the_client_wants_them_all() { + let peers_wanted = PeersWanted::All; + + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + } + + #[test] + fn it_should_allow_limiting_the_peer_list() { + let peers_wanted = PeersWanted::only(10); + + assert_eq!(peers_wanted.limit(), 10); + } + + fn maximum_as_u32() -> u32 { + u32::try_from(TORRENT_PEERS_LIMIT).unwrap() + } + + fn maximum_as_i32() -> i32 { + i32::try_from(TORRENT_PEERS_LIMIT).unwrap() + } + + #[test] + fn it_should_return_the_maximum_when_wanting_more_than_the_maximum() { + let peers_wanted = PeersWanted::only(maximum_as_u32() + 1); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + } + + #[test] + fn it_should_return_the_maximum_when_wanting_only_zero() { + let peers_wanted = PeersWanted::only(0); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + } + + #[test] + fn it_should_convert_the_peers_wanted_number_from_i32() { + // Negative. It should return the maximum + let peers_wanted: PeersWanted = (-1i32).into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // Zero. It should return the maximum + let peers_wanted: PeersWanted = 0i32.into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // Greater than the maximum. It should return the maximum + let peers_wanted: PeersWanted = (maximum_as_i32() + 1).into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // The maximum + let peers_wanted: PeersWanted = (maximum_as_i32()).into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // Smaller than the maximum + let peers_wanted: PeersWanted = (maximum_as_i32() - 1).into(); + assert_eq!(i32::try_from(peers_wanted.limit()).unwrap(), maximum_as_i32() - 1); + } + + #[test] + fn it_should_convert_the_peers_wanted_number_from_u32() { + // Zero. It should return the maximum + let peers_wanted: PeersWanted = 0u32.into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // Greater than the maximum. It should return the maximum + let peers_wanted: PeersWanted = (maximum_as_u32() + 1).into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // The maximum + let peers_wanted: PeersWanted = (maximum_as_u32()).into(); + assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); + + // Smaller than the maximum + let peers_wanted: PeersWanted = (maximum_as_u32() - 1).into(); + assert_eq!(i32::try_from(peers_wanted.limit()).unwrap(), maximum_as_i32() - 1); + } + } } } From 5fdee789a5aacdcdbfad2b2b9a5ef58919c5eaa8 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 7 Feb 2025 11:05:45 +0000 Subject: [PATCH 2/2] refactor: rename enum variant --- packages/tracker-core/src/announce_handler.rs | 45 ++++++++++++------- packages/tracker-core/src/lib.rs | 8 ++-- src/servers/http/v1/handlers/announce.rs | 2 +- src/servers/http/v1/services/announce.rs | 8 ++-- src/servers/http/v1/services/scrape.rs | 4 +- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/tracker-core/src/announce_handler.rs b/packages/tracker-core/src/announce_handler.rs index 5dd4a0291..85dd354bf 100644 --- a/packages/tracker-core/src/announce_handler.rs +++ b/packages/tracker-core/src/announce_handler.rs @@ -111,7 +111,7 @@ impl AnnounceHandler { pub enum PeersWanted { /// The peer wants as many peers as possible in the announce response. #[default] - All, + AsManyAsPossible, /// The peer only wants a certain amount of peers in the announce response. Only { amount: usize }, } @@ -124,7 +124,7 @@ impl PeersWanted { fn limit(&self) -> usize { match self { - PeersWanted::All => TORRENT_PEERS_LIMIT, + PeersWanted::AsManyAsPossible => TORRENT_PEERS_LIMIT, PeersWanted::Only { amount } => *amount, } } @@ -133,7 +133,7 @@ impl PeersWanted { impl From for PeersWanted { fn from(value: i32) -> Self { if value <= 0 { - return PeersWanted::All; + return PeersWanted::AsManyAsPossible; } // This conversion is safe because `value > 0` @@ -148,7 +148,7 @@ impl From for PeersWanted { impl From for PeersWanted { fn from(value: u32) -> Self { if value == 0 { - return PeersWanted::All; + return PeersWanted::AsManyAsPossible; } let amount = value as usize; @@ -350,7 +350,8 @@ mod tests { let mut peer = sample_peer(); - let announce_data = announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); + let announce_data = + announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.peers, vec![]); } @@ -364,11 +365,12 @@ mod tests { &sample_info_hash(), &mut previously_announced_peer, &peer_ip(), - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ); let mut peer = sample_peer_2(); - let announce_data = announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); + let announce_data = + announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.peers, vec![Arc::new(previously_announced_peer)]); } @@ -382,7 +384,7 @@ mod tests { &sample_info_hash(), &mut previously_announced_peer_1, &peer_ip(), - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ); let mut previously_announced_peer_2 = sample_peer_2(); @@ -390,7 +392,7 @@ mod tests { &sample_info_hash(), &mut previously_announced_peer_2, &peer_ip(), - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ); let mut peer = sample_peer_3(); @@ -418,7 +420,7 @@ mod tests { let mut peer = seeder(); let announce_data = - announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); + announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.stats.complete, 1); } @@ -430,7 +432,7 @@ mod tests { let mut peer = leecher(); let announce_data = - announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); + announce_handler.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.stats.incomplete, 1); } @@ -441,11 +443,20 @@ mod tests { // We have to announce with "started" event because peer does not count if peer was not previously known let mut started_peer = started_peer(); - announce_handler.announce(&sample_info_hash(), &mut started_peer, &peer_ip(), &PeersWanted::All); + announce_handler.announce( + &sample_info_hash(), + &mut started_peer, + &peer_ip(), + &PeersWanted::AsManyAsPossible, + ); let mut completed_peer = completed_peer(); - let announce_data = - announce_handler.announce(&sample_info_hash(), &mut completed_peer, &peer_ip(), &PeersWanted::All); + let announce_data = announce_handler.announce( + &sample_info_hash(), + &mut completed_peer, + &peer_ip(), + &PeersWanted::AsManyAsPossible, + ); assert_eq!(announce_data.stats.downloaded, 1); } @@ -494,11 +505,11 @@ mod tests { let mut peer = sample_peer(); peer.event = AnnounceEvent::Started; - let announce_data = announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); + let announce_data = announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.stats.downloaded, 0); peer.event = AnnounceEvent::Completed; - let announce_data = announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); + let announce_data = announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); assert_eq!(announce_data.stats.downloaded, 1); // Remove the newly updated torrent from memory @@ -533,7 +544,7 @@ mod tests { #[test] fn it_should_return_74_at_the_most_if_the_client_wants_them_all() { - let peers_wanted = PeersWanted::All; + let peers_wanted = PeersWanted::AsManyAsPossible; assert_eq!(peers_wanted.limit(), TORRENT_PEERS_LIMIT); } diff --git a/packages/tracker-core/src/lib.rs b/packages/tracker-core/src/lib.rs index 68bc48552..9334e4a02 100644 --- a/packages/tracker-core/src/lib.rs +++ b/packages/tracker-core/src/lib.rs @@ -460,7 +460,7 @@ mod tests { &info_hash, &mut complete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 10)), - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ); // Announce an "incomplete" peer for the torrent @@ -469,7 +469,7 @@ mod tests { &info_hash, &mut incomplete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 11)), - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ); // Scrape @@ -510,11 +510,11 @@ mod tests { let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); // DevSkim: ignore DS173237 let mut peer = incomplete_peer(); - announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); + announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); // Announce twice to force non zeroed swarm metadata let mut peer = complete_peer(); - announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); + announce_handler.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::AsManyAsPossible); let scrape_data = scrape_handler.scrape(&vec![info_hash]).await; diff --git a/src/servers/http/v1/handlers/announce.rs b/src/servers/http/v1/handlers/announce.rs index 4c4aa6617..f76aa7a07 100644 --- a/src/servers/http/v1/handlers/announce.rs +++ b/src/servers/http/v1/handlers/announce.rs @@ -175,7 +175,7 @@ async fn handle_announce( let mut peer = peer_from_request(announce_request, &peer_ip); let peers_wanted = match announce_request.numwant { Some(numwant) => PeersWanted::only(numwant), - None => PeersWanted::All, + None => PeersWanted::AsManyAsPossible, }; let announce_data = services::announce::invoke( diff --git a/src/servers/http/v1/services/announce.rs b/src/servers/http/v1/services/announce.rs index 64a29db5a..4de9296b3 100644 --- a/src/servers/http/v1/services/announce.rs +++ b/src/servers/http/v1/services/announce.rs @@ -195,7 +195,7 @@ mod tests { core_http_tracker_services.http_stats_event_sender.clone(), sample_info_hash(), &mut peer, - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ) .await; @@ -232,7 +232,7 @@ mod tests { http_stats_event_sender, sample_info_hash(), &mut peer, - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ) .await; } @@ -277,7 +277,7 @@ mod tests { http_stats_event_sender, sample_info_hash(), &mut peer, - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ) .await; } @@ -303,7 +303,7 @@ mod tests { http_stats_event_sender, sample_info_hash(), &mut peer, - &PeersWanted::All, + &PeersWanted::AsManyAsPossible, ) .await; } diff --git a/src/servers/http/v1/services/scrape.rs b/src/servers/http/v1/services/scrape.rs index 0a3425efe..3a2323693 100644 --- a/src/servers/http/v1/services/scrape.rs +++ b/src/servers/http/v1/services/scrape.rs @@ -182,7 +182,7 @@ mod tests { // Announce a new peer to force scrape data to contain not zeroed data let mut peer = sample_peer(); let original_peer_ip = peer.ip(); - announce_handler.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::All); + announce_handler.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible); let scrape_data = invoke(&scrape_handler, &http_stats_event_sender, &info_hashes, &original_peer_ip).await; @@ -267,7 +267,7 @@ mod tests { // Announce a new peer to force scrape data to contain not zeroed data let mut peer = sample_peer(); let original_peer_ip = peer.ip(); - announce_handler.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::All); + announce_handler.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible); let scrape_data = fake(&http_stats_event_sender, &info_hashes, &original_peer_ip).await;