From 2ddefa87fa65db43a3747a328a28510e2ca6798f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 11:45:54 +0000 Subject: [PATCH 01/12] feat: add .coverage dir to .gitignore See: https://github.com/torrust/torrust-tracker/discussions/209 --- .gitignore | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index d574298da..6b58dcb45 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,11 @@ .env -/target **/*.rs.bk -/database.json.bz2 -/database.db +/.coverage/ /.idea/ +/.vscode/launch.json /config.toml /data.db -/.vscode/launch.json +/database.db +/database.json.bz2 /storage/ - - +/target From ee5b0886ec510b1c775a7664c58ca8070c2e8074 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 11:49:40 +0000 Subject: [PATCH 02/12] docs(http): add comment for duplicate struct info --- src/tracker/torrent.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/tracker/torrent.rs b/src/tracker/torrent.rs index aa155dfac..23ca6886e 100644 --- a/src/tracker/torrent.rs +++ b/src/tracker/torrent.rs @@ -14,6 +14,8 @@ pub struct Entry { pub completed: u32, } +/// Swarm metadata dictionary in the scrape response. +/// BEP 48: #[derive(Debug, PartialEq, Default)] pub struct SwarmMetadata { pub complete: u32, // The number of active peers that have completed downloading (seeders) @@ -28,6 +30,14 @@ impl SwarmMetadata { } } +/// Swarm statistics. Alternative struct for swarm metadata in scrape response. +#[derive(Debug)] +pub struct SwamStats { + pub completed: u32, // The number of peers that have ever completed downloading + pub seeders: u32, // The number of active peers that have completed downloading (seeders) + pub leechers: u32, // The number of active peers that have not completed downloading (leechers) +} + impl Entry { #[must_use] pub fn new() -> Entry { @@ -111,13 +121,6 @@ impl Default for Entry { } } -#[derive(Debug)] -pub struct SwamStats { - pub completed: u32, - pub seeders: u32, - pub leechers: u32, -} - #[cfg(test)] mod tests { From fca5353fb825736bc2d86ed477fd25c306245a8b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 16:43:56 +0000 Subject: [PATCH 03/12] test(tracker): [#207] add test for Tracker::announce --- src/tracker/mod.rs | 388 +++++++++++++++++++++++++++++------------ src/tracker/torrent.rs | 2 +- 2 files changed, 273 insertions(+), 117 deletions(-) diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index a3e0ed4fc..c2d66244a 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -44,6 +44,7 @@ pub struct TorrentsMetrics { pub torrents: u64, } +#[derive(Debug, PartialEq, Default)] pub struct AnnounceData { pub peers: Vec, pub swam_stats: SwamStats, @@ -523,12 +524,14 @@ mod tests { mod the_tracker { use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use std::str::FromStr; use std::sync::Arc; use aquatic_udp_protocol::{AnnounceEvent, NumberOfBytes}; use crate::config::{ephemeral_configuration, Configuration}; use crate::protocol::clock::DurationSinceUnixEpoch; + use crate::protocol::info_hash::InfoHash; use crate::tracker::peer::{self, Peer}; use crate::tracker::statistics::Keeper; use crate::tracker::{TorrentsMetrics, Tracker}; @@ -555,6 +558,62 @@ mod tests { } } + fn info_hash() -> InfoHash { + "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap() + } + + // The client peer IP + fn peer_ip() -> IpAddr { + IpAddr::V4(Ipv4Addr::from_str("126.0.0.1").unwrap()) + } + + /// Sample peer whose state is not relevant for the tests + fn sample_peer() -> Peer { + complete_peer() + } + + /// Sample peer when for tests that need more than one peer + fn peer1() -> Peer { + Peer { + peer_id: peer::Id(*b"-qB00000000000000001"), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8081), + updated: DurationSinceUnixEpoch::new(1_669_397_478_934, 0), + uploaded: NumberOfBytes(0), + downloaded: NumberOfBytes(0), + left: NumberOfBytes(0), + event: AnnounceEvent::Completed, + } + } + + /// Sample peer when for tests that need more than one peer + fn peer2() -> Peer { + Peer { + peer_id: peer::Id(*b"-qB00000000000000002"), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 2)), 8082), + updated: DurationSinceUnixEpoch::new(1_669_397_478_934, 0), + uploaded: NumberOfBytes(0), + downloaded: NumberOfBytes(0), + left: NumberOfBytes(0), + event: AnnounceEvent::Completed, + } + } + + fn seeder() -> Peer { + complete_peer() + } + + fn leecher() -> Peer { + incomplete_peer() + } + + fn started_peer() -> Peer { + incomplete_peer() + } + + fn completed_peer() -> Peer { + complete_peer() + } + /// A peer that has completed downloading. fn complete_peer() -> Peer { Peer { @@ -598,181 +657,278 @@ mod tests { ); } - mod handling_an_announce_request { - mod should_assign_the_ip_to_the_peer { - - use std::net::{IpAddr, Ipv4Addr}; - - use crate::tracker::assign_ip_address_to_peer; - - #[test] - fn using_the_source_ip_instead_of_the_ip_in_the_announce_request() { - let remote_ip = IpAddr::V4(Ipv4Addr::new(126, 0, 0, 2)); + mod for_all_config_modes { - let peer_ip = assign_ip_address_to_peer(&remote_ip, None); + mod handling_an_announce_request { - assert_eq!(peer_ip, remote_ip); - } + use crate::tracker::tests::the_tracker::{info_hash, peer1, peer2, peer_ip, sample_peer, tracker_factory}; - mod and_when_the_client_ip_is_a_ipv4_loopback_ip { + mod should_assign_the_ip_to_the_peer { - use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; - use std::str::FromStr; + use std::net::{IpAddr, Ipv4Addr}; use crate::tracker::assign_ip_address_to_peer; #[test] - fn it_should_use_the_loopback_ip_if_the_tracker_does_not_have_the_external_ip_configuration() { - let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + fn using_the_source_ip_instead_of_the_ip_in_the_announce_request() { + let remote_ip = IpAddr::V4(Ipv4Addr::new(126, 0, 0, 2)); let peer_ip = assign_ip_address_to_peer(&remote_ip, None); assert_eq!(peer_ip, remote_ip); } - #[test] - fn it_should_use_the_external_tracker_ip_in_tracker_configuration_if_it_is_defined() { - let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + mod and_when_the_client_ip_is_a_ipv4_loopback_ip { + + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + use std::str::FromStr; + + use crate::tracker::assign_ip_address_to_peer; + + #[test] + fn it_should_use_the_loopback_ip_if_the_tracker_does_not_have_the_external_ip_configuration() { + let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + + let peer_ip = assign_ip_address_to_peer(&remote_ip, None); + + assert_eq!(peer_ip, remote_ip); + } + + #[test] + fn it_should_use_the_external_tracker_ip_in_tracker_configuration_if_it_is_defined() { + let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + + let tracker_external_ip = IpAddr::V4(Ipv4Addr::from_str("126.0.0.1").unwrap()); + + let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + + assert_eq!(peer_ip, tracker_external_ip); + } + + #[test] + fn it_should_use_the_external_ip_in_the_tracker_configuration_if_it_is_defined_even_if_the_external_ip_is_an_ipv6_ip( + ) { + let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); - let tracker_external_ip = IpAddr::V4(Ipv4Addr::from_str("126.0.0.1").unwrap()); + let tracker_external_ip = + IpAddr::V6(Ipv6Addr::from_str("2345:0425:2CA1:0000:0000:0567:5673:23b5").unwrap()); - let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); - assert_eq!(peer_ip, tracker_external_ip); + assert_eq!(peer_ip, tracker_external_ip); + } } - #[test] - fn it_should_use_the_external_ip_in_the_tracker_configuration_if_it_is_defined_even_if_the_external_ip_is_an_ipv6_ip( - ) { - let remote_ip = IpAddr::V4(Ipv4Addr::LOCALHOST); + mod and_when_client_ip_is_a_ipv6_loopback_ip { + + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + use std::str::FromStr; + + use crate::tracker::assign_ip_address_to_peer; + + #[test] + fn it_should_use_the_loopback_ip_if_the_tracker_does_not_have_the_external_ip_configuration() { + let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); + + let peer_ip = assign_ip_address_to_peer(&remote_ip, None); + + assert_eq!(peer_ip, remote_ip); + } - let tracker_external_ip = - IpAddr::V6(Ipv6Addr::from_str("2345:0425:2CA1:0000:0000:0567:5673:23b5").unwrap()); + #[test] + fn it_should_use_the_external_ip_in_tracker_configuration_if_it_is_defined() { + let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); - let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + let tracker_external_ip = + IpAddr::V6(Ipv6Addr::from_str("2345:0425:2CA1:0000:0000:0567:5673:23b5").unwrap()); - assert_eq!(peer_ip, tracker_external_ip); + let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + + assert_eq!(peer_ip, tracker_external_ip); + } + + #[test] + fn it_should_use_the_external_ip_in_the_tracker_configuration_if_it_is_defined_even_if_the_external_ip_is_an_ipv4_ip( + ) { + let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); + + let tracker_external_ip = IpAddr::V4(Ipv4Addr::from_str("126.0.0.1").unwrap()); + + let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + + assert_eq!(peer_ip, tracker_external_ip); + } } } - mod and_when_client_ip_is_a_ipv6_loopback_ip { + #[tokio::test] + async fn it_should_return_the_announce_data_with_an_empty_peer_list_when_it_is_the_first_announced_peer() { + let tracker = tracker_factory(); - use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; - use std::str::FromStr; + let mut peer = sample_peer(); - use crate::tracker::assign_ip_address_to_peer; + let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; - #[test] - fn it_should_use_the_loopback_ip_if_the_tracker_does_not_have_the_external_ip_configuration() { - let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); + assert_eq!(announce_data.peers, vec![]); + } - let peer_ip = assign_ip_address_to_peer(&remote_ip, None); + #[tokio::test] + async fn it_should_return_the_announce_data_with_the_previously_announced_peers() { + let tracker = tracker_factory(); - assert_eq!(peer_ip, remote_ip); + let mut previously_announced_peer = peer1(); + tracker + .announce(&info_hash(), &mut previously_announced_peer, &peer_ip()) + .await; + + let mut peer = peer2(); + let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + + assert_eq!(announce_data.peers, vec![previously_announced_peer]); + } + + mod it_should_update_the_swarm_stats_for_the_torrent { + + use crate::tracker::tests::the_tracker::{ + completed_peer, info_hash, leecher, peer_ip, seeder, started_peer, tracker_factory, + }; + + #[tokio::test] + async fn when_the_peer_is_a_seeder() { + let tracker = tracker_factory(); + + let mut peer = seeder(); + + let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + + assert_eq!(announce_data.swam_stats.seeders, 1); } - #[test] - fn it_should_use_the_external_ip_in_tracker_configuration_if_it_is_defined() { - let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); + #[tokio::test] + async fn when_the_peer_is_a_leecher() { + let tracker = tracker_factory(); - let tracker_external_ip = - IpAddr::V6(Ipv6Addr::from_str("2345:0425:2CA1:0000:0000:0567:5673:23b5").unwrap()); + let mut peer = leecher(); - let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; - assert_eq!(peer_ip, tracker_external_ip); + assert_eq!(announce_data.swam_stats.leechers, 1); } - #[test] - fn it_should_use_the_external_ip_in_the_tracker_configuration_if_it_is_defined_even_if_the_external_ip_is_an_ipv4_ip( - ) { - let remote_ip = IpAddr::V6(Ipv6Addr::LOCALHOST); + #[tokio::test] + async fn when_a_previously_announced_started_peer_has_completed_downloading() { + let tracker = tracker_factory(); - let tracker_external_ip = IpAddr::V4(Ipv4Addr::from_str("126.0.0.1").unwrap()); + // We have to announce with "started" event because peer does not count if peer was not previously known + let mut started_peer = started_peer(); + tracker.announce(&info_hash(), &mut started_peer, &peer_ip()).await; - let peer_ip = assign_ip_address_to_peer(&remote_ip, Some(tracker_external_ip)); + let mut completed_peer = completed_peer(); + let announce_data = tracker.announce(&info_hash(), &mut completed_peer, &peer_ip()).await; - assert_eq!(peer_ip, tracker_external_ip); + assert_eq!(announce_data.swam_stats.completed, 1); } } } - } - mod handling_a_scrape_request { + mod handling_a_scrape_request { - use std::net::{IpAddr, Ipv4Addr}; + use std::net::{IpAddr, Ipv4Addr}; - use crate::protocol::info_hash::InfoHash; - use crate::tracker::tests::the_tracker::{complete_peer, incomplete_peer, tracker_factory}; - use crate::tracker::{ScrapeData, SwarmMetadata}; + use crate::protocol::info_hash::InfoHash; + use crate::tracker::tests::the_tracker::{complete_peer, incomplete_peer, tracker_factory}; + use crate::tracker::{ScrapeData, SwarmMetadata}; - #[tokio::test] - async fn it_should_return_a_zeroed_swarm_metadata_for_the_requested_file_if_the_tracker_does_not_have_that_torrent() { - let tracker = tracker_factory(); + #[tokio::test] + async fn it_should_return_a_zeroed_swarm_metadata_for_the_requested_file_if_the_tracker_does_not_have_that_torrent( + ) { + let tracker = tracker_factory(); - let info_hashes = vec!["3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap()]; + let info_hashes = vec!["3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap()]; - let scrape_data = tracker.scrape(&info_hashes).await; + let scrape_data = tracker.scrape(&info_hashes).await; - let mut expected_scrape_data = ScrapeData::empty(); + let mut expected_scrape_data = ScrapeData::empty(); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); + expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); - assert_eq!(scrape_data, expected_scrape_data); - } + assert_eq!(scrape_data, expected_scrape_data); + } - #[tokio::test] - async fn it_should_return_the_swarm_metadata_for_the_requested_file_if_the_tracker_has_that_torrent() { - let tracker = tracker_factory(); - - let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); - - // Announce a "complete" peer for the torrent - let mut complete_peer = complete_peer(); - tracker - .announce(&info_hash, &mut complete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 10))) - .await; - - // Announce an "incomplete" peer for the torrent - let mut incomplete_peer = incomplete_peer(); - tracker - .announce(&info_hash, &mut incomplete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 11))) - .await; - - // Scrape - let scrape_data = tracker.scrape(&vec![info_hash]).await; - - // The expected swarm metadata for the file - let mut expected_scrape_data = ScrapeData::empty(); - expected_scrape_data.add_file( - &info_hash, - SwarmMetadata { - complete: 0, // the "complete" peer does not count because it was not previously known - downloaded: 0, - incomplete: 1, // the "incomplete" peer we have just announced - }, - ); - - assert_eq!(scrape_data, expected_scrape_data); - } + #[tokio::test] + async fn it_should_return_the_swarm_metadata_for_the_requested_file_if_the_tracker_has_that_torrent() { + let tracker = tracker_factory(); + + let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); + + // Announce a "complete" peer for the torrent + let mut complete_peer = complete_peer(); + tracker + .announce(&info_hash, &mut complete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 10))) + .await; + + // Announce an "incomplete" peer for the torrent + let mut incomplete_peer = incomplete_peer(); + tracker + .announce(&info_hash, &mut incomplete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 11))) + .await; + + // Scrape + let scrape_data = tracker.scrape(&vec![info_hash]).await; + + // The expected swarm metadata for the file + let mut expected_scrape_data = ScrapeData::empty(); + expected_scrape_data.add_file( + &info_hash, + SwarmMetadata { + complete: 0, // the "complete" peer does not count because it was not previously known + downloaded: 0, + incomplete: 1, // the "incomplete" peer we have just announced + }, + ); + + assert_eq!(scrape_data, expected_scrape_data); + } - #[tokio::test] - async fn it_should_allow_scraping_for_multiple_torrents() { - let tracker = tracker_factory(); + #[tokio::test] + async fn it_should_allow_scraping_for_multiple_torrents() { + let tracker = tracker_factory(); - let info_hashes = vec![ - "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(), - "99c82bb73505a3c0b453f9fa0e881d6e5a32a0c1".parse::().unwrap(), - ]; + let info_hashes = vec![ + "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(), + "99c82bb73505a3c0b453f9fa0e881d6e5a32a0c1".parse::().unwrap(), + ]; - let scrape_data = tracker.scrape(&info_hashes).await; + let scrape_data = tracker.scrape(&info_hashes).await; - let mut expected_scrape_data = ScrapeData::empty(); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[1]); + let mut expected_scrape_data = ScrapeData::empty(); + expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); + expected_scrape_data.add_file_with_no_metadata(&info_hashes[1]); - assert_eq!(scrape_data, expected_scrape_data); + assert_eq!(scrape_data, expected_scrape_data); + } } } + + mod configured_as_whitelisted { + + mod handling_an_announce_request {} + + mod handling_an_scrape_request {} + } + + mod configured_as_private { + + mod handling_an_announce_request {} + + mod handling_an_scrape_request {} + } + + mod configured_as_private_and_whitelisted { + + mod handling_an_announce_request {} + + mod handling_an_scrape_request {} + } } } diff --git a/src/tracker/torrent.rs b/src/tracker/torrent.rs index 23ca6886e..3362234f0 100644 --- a/src/tracker/torrent.rs +++ b/src/tracker/torrent.rs @@ -31,7 +31,7 @@ impl SwarmMetadata { } /// Swarm statistics. Alternative struct for swarm metadata in scrape response. -#[derive(Debug)] +#[derive(Debug, PartialEq, Default)] pub struct SwamStats { pub completed: u32, // The number of peers that have ever completed downloading pub seeders: u32, // The number of active peers that have completed downloading (seeders) From d50372fbf064413e2d05aac9a5be2d4b13136083 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 16:44:55 +0000 Subject: [PATCH 04/12] feat(cargo): add cargo alias to generate coverage reports --- .cargo/config.toml | 3 +++ cSpell.json | 1 + 2 files changed, 4 insertions(+) create mode 100644 .cargo/config.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..2fb2fe92d --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,3 @@ +[alias] +cov = "llvm-cov --lcov --output-path=./coverage/lcov.info" +cov-html = "llvm-cov --html" diff --git a/cSpell.json b/cSpell.json index b8aceb568..4a9b11ce9 100644 --- a/cSpell.json +++ b/cSpell.json @@ -31,6 +31,7 @@ "infohashes", "infoschema", "intervali", + "lcov", "leecher", "leechers", "libtorrent", From 1e7eff59d35e50cea6d110bdf7528f02af3f011d Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 17:14:26 +0000 Subject: [PATCH 05/12] docs(tracker): add code-review --- src/tracker/mod.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index c2d66244a..d638f6601 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -120,6 +120,17 @@ impl Tracker { // a tuple with the new peer and the announce data: (Peer, AnnounceData). // It could even be a different struct: `StoredPeer` or `PublicPeer`. + // code-review: in the `scrape` function we perform an authorization check. + // We check if the torrent is whitelisted. Should we also check authorization here? + // I think so because the `Tracker` has the responsibility for checking authentication and authorization. + // The `Tracker` has delegated that responsibility to the handlers + // (because we want to return a friendly error response) but that does not mean we should + // double-check authorization at this domain level too. + // I would propose to return a `Result` here. + // Besides, regarding authentication the `Tracker` is also responsible for authentication but + // we are actually handling authentication at the handlers level. So I would extract that + // responsibility into another authentication service. + peer.change_ip(&assign_ip_address_to_peer(remote_client_ip, self.config.get_ext_ip())); let swam_stats = self.update_torrent_with_peer_and_get_stats(info_hash, peer).await; @@ -614,7 +625,9 @@ mod tests { complete_peer() } - /// A peer that has completed downloading. + /// A peer that counts as `complete` is swarm metadata + /// IMPORTANT!: it only counts if the it has been announce at least once before + /// announcing the `AnnounceEvent::Completed` event. fn complete_peer() -> Peer { Peer { peer_id: peer::Id(*b"-qB00000000000000000"), @@ -627,7 +640,7 @@ mod tests { } } - /// A peer that has NOT completed downloading. + /// A peer that counts as `incomplete` is swarm metadata fn incomplete_peer() -> Peer { Peer { peer_id: peer::Id(*b"-qB00000000000000000"), From ed58a32ce32d0644804e0f97120bf5e32d4b3c82 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 21:23:05 +0000 Subject: [PATCH 06/12] test(tracker): [#207] add test for Tracker::scrape --- .../axum_implementation/services/scrape.rs | 4 +- src/tracker/mod.rs | 146 ++++++++++++------ 2 files changed, 99 insertions(+), 51 deletions(-) diff --git a/src/http/axum_implementation/services/scrape.rs b/src/http/axum_implementation/services/scrape.rs index 30f00a47b..923acf3c4 100644 --- a/src/http/axum_implementation/services/scrape.rs +++ b/src/http/axum_implementation/services/scrape.rs @@ -15,11 +15,9 @@ pub async fn invoke(tracker: &Arc, info_hashes: &Vec, origina /// When the peer is not authenticated and the tracker is running in `private` mode, /// the tracker returns empty stats for all the torrents. pub async fn fake_invoke(tracker: &Arc, info_hashes: &Vec, original_peer_ip: &IpAddr) -> ScrapeData { - let scrape_data = tracker.empty_scrape_for(info_hashes); - send_scrape_event(original_peer_ip, tracker).await; - scrape_data + ScrapeData::zeroed(info_hashes) } async fn send_scrape_event(original_peer_ip: &IpAddr, tracker: &Arc) { diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index d638f6601..6fa1d38fc 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -64,12 +64,23 @@ impl ScrapeData { Self { files } } + #[must_use] + pub fn zeroed(info_hashes: &Vec) -> Self { + let mut scrape_data = Self::empty(); + + for info_hash in info_hashes { + scrape_data.add_file(info_hash, SwarmMetadata::zeroed()); + } + + scrape_data + } + pub fn add_file(&mut self, info_hash: &InfoHash, swarm_metadata: SwarmMetadata) { self.files.insert(*info_hash, swarm_metadata); } - pub fn add_file_with_no_metadata(&mut self, info_hash: &InfoHash) { - self.files.insert(*info_hash, SwarmMetadata::default()); + pub fn add_file_with_zeroed_metadata(&mut self, info_hash: &InfoHash) { + self.files.insert(*info_hash, SwarmMetadata::zeroed()); } } @@ -162,17 +173,6 @@ impl Tracker { scrape_data } - // It return empty swarm metadata for all the infohashes. - pub fn empty_scrape_for(&self, info_hashes: &Vec) -> ScrapeData { - let mut scrape_data = ScrapeData::empty(); - - for info_hash in info_hashes { - scrape_data.add_file(info_hash, SwarmMetadata::default()); - } - - scrape_data - } - async fn get_swarm_metadata(&self, info_hash: &InfoHash) -> SwarmMetadata { let torrents = self.get_torrents().await; match torrents.get(info_hash) { @@ -543,25 +543,31 @@ mod tests { use crate::config::{ephemeral_configuration, Configuration}; use crate::protocol::clock::DurationSinceUnixEpoch; use crate::protocol::info_hash::InfoHash; + use crate::tracker::mode::Mode; use crate::tracker::peer::{self, Peer}; use crate::tracker::statistics::Keeper; use crate::tracker::{TorrentsMetrics, Tracker}; - pub fn tracker_configuration() -> Arc { - Arc::new(ephemeral_configuration()) + pub fn public_tracker() -> Tracker { + let mut configuration = ephemeral_configuration(); + configuration.mode = Mode::Public; + tracker_factory(configuration) } - pub fn tracker_factory() -> Tracker { - // code-review: the tracker initialization is duplicated in many places. Consider make this function public. + pub fn whitelisted_tracker() -> Tracker { + let mut configuration = ephemeral_configuration(); + configuration.mode = Mode::Listed; + tracker_factory(configuration) + } - // Configuration - let configuration = tracker_configuration(); + pub fn tracker_factory(configuration: Configuration) -> Tracker { + // code-review: the tracker initialization is duplicated in many places. Consider make this function public. // Initialize stats tracker let (stats_event_sender, stats_repository) = Keeper::new_active_instance(); // Initialize Torrust tracker - match Tracker::new(&configuration, Some(stats_event_sender), stats_repository) { + match Tracker::new(&Arc::new(configuration), Some(stats_event_sender), stats_repository) { Ok(tracker) => tracker, Err(error) => { panic!("{}", error) @@ -569,7 +575,7 @@ mod tests { } } - fn info_hash() -> InfoHash { + fn sample_info_hash() -> InfoHash { "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap() } @@ -584,7 +590,7 @@ mod tests { } /// Sample peer when for tests that need more than one peer - fn peer1() -> Peer { + fn sample_peer_1() -> Peer { Peer { peer_id: peer::Id(*b"-qB00000000000000001"), peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8081), @@ -597,7 +603,7 @@ mod tests { } /// Sample peer when for tests that need more than one peer - fn peer2() -> Peer { + fn sample_peer_2() -> Peer { Peer { peer_id: peer::Id(*b"-qB00000000000000002"), peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 2)), 8082), @@ -655,7 +661,7 @@ mod tests { #[tokio::test] async fn should_collect_torrent_metrics() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let torrents_metrics = tracker.get_torrents_metrics().await; @@ -674,7 +680,9 @@ mod tests { mod handling_an_announce_request { - use crate::tracker::tests::the_tracker::{info_hash, peer1, peer2, peer_ip, sample_peer, tracker_factory}; + use crate::tracker::tests::the_tracker::{ + peer_ip, public_tracker, sample_info_hash, sample_peer, sample_peer_1, sample_peer_2, + }; mod should_assign_the_ip_to_the_peer { @@ -776,26 +784,26 @@ mod tests { #[tokio::test] async fn it_should_return_the_announce_data_with_an_empty_peer_list_when_it_is_the_first_announced_peer() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let mut peer = sample_peer(); - let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; assert_eq!(announce_data.peers, vec![]); } #[tokio::test] async fn it_should_return_the_announce_data_with_the_previously_announced_peers() { - let tracker = tracker_factory(); + let tracker = public_tracker(); - let mut previously_announced_peer = peer1(); + let mut previously_announced_peer = sample_peer_1(); tracker - .announce(&info_hash(), &mut previously_announced_peer, &peer_ip()) + .announce(&sample_info_hash(), &mut previously_announced_peer, &peer_ip()) .await; - let mut peer = peer2(); - let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + let mut peer = sample_peer_2(); + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; assert_eq!(announce_data.peers, vec![previously_announced_peer]); } @@ -803,41 +811,41 @@ mod tests { mod it_should_update_the_swarm_stats_for_the_torrent { use crate::tracker::tests::the_tracker::{ - completed_peer, info_hash, leecher, peer_ip, seeder, started_peer, tracker_factory, + completed_peer, leecher, peer_ip, public_tracker, sample_info_hash, seeder, started_peer, }; #[tokio::test] async fn when_the_peer_is_a_seeder() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let mut peer = seeder(); - let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; assert_eq!(announce_data.swam_stats.seeders, 1); } #[tokio::test] async fn when_the_peer_is_a_leecher() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let mut peer = leecher(); - let announce_data = tracker.announce(&info_hash(), &mut peer, &peer_ip()).await; + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; assert_eq!(announce_data.swam_stats.leechers, 1); } #[tokio::test] async fn when_a_previously_announced_started_peer_has_completed_downloading() { - let tracker = tracker_factory(); + let tracker = public_tracker(); // We have to announce with "started" event because peer does not count if peer was not previously known let mut started_peer = started_peer(); - tracker.announce(&info_hash(), &mut started_peer, &peer_ip()).await; + tracker.announce(&sample_info_hash(), &mut started_peer, &peer_ip()).await; let mut completed_peer = completed_peer(); - let announce_data = tracker.announce(&info_hash(), &mut completed_peer, &peer_ip()).await; + let announce_data = tracker.announce(&sample_info_hash(), &mut completed_peer, &peer_ip()).await; assert_eq!(announce_data.swam_stats.completed, 1); } @@ -849,13 +857,13 @@ mod tests { use std::net::{IpAddr, Ipv4Addr}; use crate::protocol::info_hash::InfoHash; - use crate::tracker::tests::the_tracker::{complete_peer, incomplete_peer, tracker_factory}; + use crate::tracker::tests::the_tracker::{complete_peer, incomplete_peer, public_tracker}; use crate::tracker::{ScrapeData, SwarmMetadata}; #[tokio::test] async fn it_should_return_a_zeroed_swarm_metadata_for_the_requested_file_if_the_tracker_does_not_have_that_torrent( ) { - let tracker = tracker_factory(); + let tracker = public_tracker(); let info_hashes = vec!["3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap()]; @@ -863,14 +871,14 @@ mod tests { let mut expected_scrape_data = ScrapeData::empty(); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); + expected_scrape_data.add_file_with_zeroed_metadata(&info_hashes[0]); assert_eq!(scrape_data, expected_scrape_data); } #[tokio::test] async fn it_should_return_the_swarm_metadata_for_the_requested_file_if_the_tracker_has_that_torrent() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); @@ -905,7 +913,7 @@ mod tests { #[tokio::test] async fn it_should_allow_scraping_for_multiple_torrents() { - let tracker = tracker_factory(); + let tracker = public_tracker(); let info_hashes = vec![ "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(), @@ -915,8 +923,8 @@ mod tests { let scrape_data = tracker.scrape(&info_hashes).await; let mut expected_scrape_data = ScrapeData::empty(); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[0]); - expected_scrape_data.add_file_with_no_metadata(&info_hashes[1]); + expected_scrape_data.add_file_with_zeroed_metadata(&info_hashes[0]); + expected_scrape_data.add_file_with_zeroed_metadata(&info_hashes[1]); assert_eq!(scrape_data, expected_scrape_data); } @@ -927,7 +935,49 @@ mod tests { mod handling_an_announce_request {} - mod handling_an_scrape_request {} + mod handling_an_scrape_request { + + use crate::protocol::info_hash::InfoHash; + use crate::tracker::tests::the_tracker::{ + complete_peer, incomplete_peer, peer_ip, sample_info_hash, whitelisted_tracker, + }; + use crate::tracker::torrent::SwarmMetadata; + use crate::tracker::ScrapeData; + + #[test] + fn it_should_be_able_to_build_a_zeroed_scrape_data_for_a_list_of_info_hashes() { + // Zeroed scrape data is used when the authentication for the scrape request fails. + + let sample_info_hash = sample_info_hash(); + + let mut expected_scrape_data = ScrapeData::empty(); + expected_scrape_data.add_file_with_zeroed_metadata(&sample_info_hash); + + assert_eq!(ScrapeData::zeroed(&vec![sample_info_hash]), expected_scrape_data); + } + + #[tokio::test] + async fn it_should_return_the_zeroed_swarm_metadata_for_the_requested_file_if_it_is_not_whitelisted() { + let tracker = whitelisted_tracker(); + + let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); + + let mut peer = incomplete_peer(); + tracker.announce(&info_hash, &mut peer, &peer_ip()).await; + + // Announce twice to force non zeroed swarm metadata + let mut peer = complete_peer(); + tracker.announce(&info_hash, &mut peer, &peer_ip()).await; + + let scrape_data = tracker.scrape(&vec![info_hash]).await; + + // The expected zeroed swarm metadata for the file + let mut expected_scrape_data = ScrapeData::empty(); + expected_scrape_data.add_file(&info_hash, SwarmMetadata::zeroed()); + + assert_eq!(scrape_data, expected_scrape_data); + } + } } mod configured_as_private { From d1a7b7fb2493877cb6db814d54fb5709b8f7d5be Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 2 Mar 2023 21:39:05 +0000 Subject: [PATCH 07/12] fix(cargo): fix output path in cargo allias --- .cargo/config.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 2fb2fe92d..e3d31cf7f 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,3 @@ [alias] -cov = "llvm-cov --lcov --output-path=./coverage/lcov.info" +cov = "llvm-cov --lcov --output-path=./.coverage/lcov.info" cov-html = "llvm-cov --html" From fad683425c284a9c29ee6fa7f597e30767668950 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 3 Mar 2023 13:29:45 +0000 Subject: [PATCH 08/12] test(tracker): [#207] add tests for HTTP authentication in Tracker The `Tracker` is responsible for the authentication in the HTTP protocol. --- src/tracker/auth.rs | 3 ++ src/tracker/mod.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/src/tracker/auth.rs b/src/tracker/auth.rs index 22f734e48..01de7a619 100644 --- a/src/tracker/auth.rs +++ b/src/tracker/auth.rs @@ -62,6 +62,9 @@ pub fn verify(auth_key: &ExpiringKey) -> Result<(), Error> { #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] pub struct ExpiringKey { pub id: KeyId, + // todo: we can remove the `Option`. An `ExpiringKey` that does not expire + // is a `KeyId`. In other words, all `ExpiringKeys` must have an + // expiration time. pub valid_until: Option, } diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 6fa1d38fc..b8dadab8f 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -199,6 +199,7 @@ impl Tracker { /// /// Will panic if key cannot be converted into a valid `KeyId`. pub async fn remove_auth_key(&self, key: &str) -> Result<(), databases::error::Error> { + // todo: change argument `key: &str` to `key_id: &KeyId` self.database.remove_key_from_keys(key).await?; self.keys.write().await.remove(&key.parse::().unwrap()); Ok(()) @@ -208,6 +209,8 @@ impl Tracker { /// /// Will return a `key::Error` if unable to get any `auth_key`. pub async fn verify_auth_key(&self, key_id: &KeyId) -> Result<(), auth::Error> { + // code-review: this function is public only because it's used in a test. + // We should change the test and make it private. match self.keys.read().await.get(key_id) { None => Err(auth::Error::UnableToReadKey { location: Location::caller(), @@ -319,6 +322,12 @@ impl Tracker { /// /// Will return a `torrent::Error::TorrentNotWhitelisted` if the the Tracker is in listed mode and the `info_hash` is not whitelisted. pub async fn authenticate_request(&self, info_hash: &InfoHash, key: &Option) -> Result<(), Error> { + // todo: this is a deprecated method. + // We're splitting authentication and authorization responsibilities. + // Use `authenticate` and `authorize` instead. + + // Authentication + // no authentication needed in public mode if self.is_public() { return Ok(()); @@ -343,6 +352,8 @@ impl Tracker { } } + // Authorization + // check if info_hash is whitelisted if self.is_whitelisted() && !self.is_info_hash_whitelisted(info_hash).await { return Err(Error::TorrentNotWhitelisted { @@ -554,6 +565,12 @@ mod tests { tracker_factory(configuration) } + pub fn private_tracker() -> Tracker { + let mut configuration = ephemeral_configuration(); + configuration.mode = Mode::Private; + tracker_factory(configuration) + } + pub fn whitelisted_tracker() -> Tracker { let mut configuration = ephemeral_configuration(); configuration.mode = Mode::Listed; @@ -982,6 +999,92 @@ mod tests { mod configured_as_private { + mod handling_authentication { + use std::str::FromStr; + use std::time::Duration; + + use crate::tracker::auth; + use crate::tracker::tests::the_tracker::private_tracker; + + #[tokio::test] + async fn it_should_generate_the_expiring_authentication_keys() { + let tracker = private_tracker(); + + let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + + assert_eq!(key.valid_until.unwrap(), Duration::from_secs(100)); + } + + #[tokio::test] + async fn it_should_authenticate_a_peer_by_using_a_key() { + let tracker = private_tracker(); + + let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + + let result = tracker.authenticate(&key.id()).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn it_should_fail_authenticating_a_peer_when_it_uses_an_unregistered_key() { + let tracker = private_tracker(); + + let unregistered_key_id = auth::KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + let result = tracker.authenticate(&unregistered_key_id).await; + + assert!(result.is_err()); + } + + #[tokio::test] + async fn it_should_verify_a_valid_authentication_key() { + // todo: this should not be tested directly because + // `verify_auth_key` should be a private method. + let tracker = private_tracker(); + + let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + + assert!(tracker.verify_auth_key(&key.id()).await.is_ok()); + } + + #[tokio::test] + async fn it_should_fail_verifying_an_unregistered_authentication_key() { + let tracker = private_tracker(); + + let unregistered_key_id = auth::KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + assert!(tracker.verify_auth_key(&unregistered_key_id).await.is_err()); + } + + #[tokio::test] + async fn it_should_remove_an_authentication_key() { + let tracker = private_tracker(); + + let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + + let result = tracker.remove_auth_key(&key.id().to_string()).await; + + assert!(result.is_ok()); + assert!(tracker.verify_auth_key(&key.id()).await.is_err()); + } + + #[tokio::test] + async fn it_should_load_authentication_keys_from_the_database() { + let tracker = private_tracker(); + + let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + + // Remove the newly generated key in memory + tracker.keys.write().await.remove(&key.id()); + + let result = tracker.load_keys().await; + + assert!(result.is_ok()); + assert!(tracker.verify_auth_key(&key.id()).await.is_ok()); + } + } + mod handling_an_announce_request {} mod handling_an_scrape_request {} From 40ff2498cf3045512daa308021fffd15b2729299 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 3 Mar 2023 13:53:23 +0000 Subject: [PATCH 09/12] test(tracker): [#207] add test for authorization (whitelist) in Tracker --- src/tracker/mod.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index b8dadab8f..e16fefa4f 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -950,6 +950,33 @@ mod tests { mod configured_as_whitelisted { + mod handling_authorization { + use crate::tracker::tests::the_tracker::{sample_info_hash, whitelisted_tracker}; + + #[tokio::test] + async fn it_should_authorize_the_announce_and_scrape_actions_on_whitelisted_torrents() { + let tracker = whitelisted_tracker(); + + let info_hash = sample_info_hash(); + + let result = tracker.add_torrent_to_whitelist(&info_hash).await; + assert!(result.is_ok()); + + let result = tracker.authorize(&info_hash).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn it_should_not_authorize_the_announce_and_scrape_actions_on_not_whitelisted_torrents() { + let tracker = whitelisted_tracker(); + + let info_hash = sample_info_hash(); + + let result = tracker.authorize(&info_hash).await; + assert!(result.is_err()); + } + } + mod handling_an_announce_request {} mod handling_an_scrape_request { From af949af4b8d11826836a3e1153cb542359bcd404 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 3 Mar 2023 16:07:47 +0000 Subject: [PATCH 10/12] test(tracker): [#207] add tests for torrent persistence in Tracker --- src/tracker/mod.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index e16fefa4f..5de9d6f3c 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -404,6 +404,7 @@ impl Tracker { /// Will return a `database::Error` if unable to load the list of `persistent_torrents` from the database. pub async fn load_persistent_torrents(&self) -> Result<(), databases::error::Error> { let persistent_torrents = self.database.load_persistent_torrents().await?; + let mut torrents = self.torrents.write().await; for (info_hash, completed) in persistent_torrents { @@ -577,6 +578,12 @@ mod tests { tracker_factory(configuration) } + pub fn tracker_persisting_torrents_in_database() -> Tracker { + let mut configuration = ephemeral_configuration(); + configuration.persistent_torrent_completed_stat = true; + tracker_factory(configuration) + } + pub fn tracker_factory(configuration: Configuration) -> Tracker { // code-review: the tracker initialization is duplicated in many places. Consider make this function public. @@ -1123,5 +1130,47 @@ mod tests { mod handling_an_scrape_request {} } + + mod handling_torrent_persistence { + use aquatic_udp_protocol::AnnounceEvent; + + use crate::tracker::tests::the_tracker::{sample_info_hash, sample_peer, tracker_persisting_torrents_in_database}; + + #[tokio::test] + async fn it_should_persist_the_number_of_completed_peers_for_all_torrents_into_the_database() { + let tracker = tracker_persisting_torrents_in_database(); + + let info_hash = sample_info_hash(); + + let mut peer = sample_peer(); + + peer.event = AnnounceEvent::Started; + let swarm_stats = tracker.update_torrent_with_peer_and_get_stats(&info_hash, &peer).await; + assert_eq!(swarm_stats.completed, 0); + + peer.event = AnnounceEvent::Completed; + let swarm_stats = tracker.update_torrent_with_peer_and_get_stats(&info_hash, &peer).await; + assert_eq!(swarm_stats.completed, 1); + + let torrents = tracker.get_all_torrent_peers(&info_hash).await; + assert_eq!(torrents.len(), 1); + + // Remove the newly updated torrent from memory + tracker.torrents.write().await.remove(&info_hash); + + tracker.load_persistent_torrents().await.unwrap(); + + let torrents = tracker.get_torrents().await; + assert!(torrents.contains_key(&info_hash)); + + let torrent_entry = torrents.get(&info_hash).unwrap(); + + // It persists the number of completed peers. + assert_eq!(torrent_entry.completed, 1); + + // It does not persist the peers + assert!(torrent_entry.peers.is_empty()); + } + } } } From 7fb92b5c0ac390465159e1567dc1ff92b9e15ced Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 3 Mar 2023 17:25:11 +0000 Subject: [PATCH 11/12] test(tracker): [#207] add tests for whitelist in Tracker --- src/apis/handlers.rs | 4 +- src/setup.rs | 7 ++- src/tracker/mod.rs | 114 ++++++++++++++++++++++++++++++++++++++--- src/tracker/torrent.rs | 4 +- 4 files changed, 116 insertions(+), 13 deletions(-) diff --git a/src/apis/handlers.rs b/src/apis/handlers.rs index 38959edbe..f7b5e562c 100644 --- a/src/apis/handlers.rs +++ b/src/apis/handlers.rs @@ -86,7 +86,7 @@ pub async fn remove_torrent_from_whitelist_handler( } pub async fn reload_whitelist_handler(State(tracker): State>) -> Response { - match tracker.load_whitelist().await { + match tracker.load_whitelist_from_database().await { Ok(_) => ok_response(), Err(e) => failed_to_reload_whitelist_response(e), } @@ -117,7 +117,7 @@ pub async fn delete_auth_key_handler( } pub async fn reload_keys_handler(State(tracker): State>) -> Response { - match tracker.load_keys().await { + match tracker.load_keys_from_database().await { Ok(_) => ok_response(), Err(e) => failed_to_reload_keys_response(e), } diff --git a/src/setup.rs b/src/setup.rs index 3461667cc..98d311178 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -16,13 +16,16 @@ pub async fn setup(config: &Configuration, tracker: Arc) -> Ve // Load peer keys if tracker.is_private() { - tracker.load_keys().await.expect("Could not retrieve keys from database."); + tracker + .load_keys_from_database() + .await + .expect("Could not retrieve keys from database."); } // Load whitelisted torrents if tracker.is_whitelisted() { tracker - .load_whitelist() + .load_whitelist_from_database() .await .expect("Could not load whitelist from database."); } diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 5de9d6f3c..3048ded35 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -38,6 +38,9 @@ pub struct Tracker { #[derive(Debug, PartialEq, Default)] pub struct TorrentsMetrics { + // code-review: consider using `SwamStats` for + // `seeders`, `completed`, and `leechers` attributes. + // pub swam_stats: SwamStats; pub seeders: u64, pub completed: u64, pub leechers: u64, @@ -223,7 +226,7 @@ impl Tracker { /// # Errors /// /// Will return a `database::Error` if unable to `load_keys` from the database. - pub async fn load_keys(&self) -> Result<(), databases::error::Error> { + pub async fn load_keys_from_database(&self) -> Result<(), databases::error::Error> { let keys_from_database = self.database.load_keys().await?; let mut keys = self.keys.write().await; @@ -301,7 +304,7 @@ impl Tracker { /// # Errors /// /// Will return a `database::Error` if unable to load the list whitelisted `info_hash`s from the database. - pub async fn load_whitelist(&self) -> Result<(), databases::error::Error> { + pub async fn load_whitelist_from_database(&self) -> Result<(), databases::error::Error> { let whitelisted_torrents_from_database = self.database.load_whitelist().await?; let mut whitelist = self.whitelist.write().await; @@ -402,7 +405,7 @@ impl Tracker { /// # Errors /// /// Will return a `database::Error` if unable to load the list of `persistent_torrents` from the database. - pub async fn load_persistent_torrents(&self) -> Result<(), databases::error::Error> { + pub async fn load_torrents_from_database(&self) -> Result<(), databases::error::Error> { let persistent_torrents = self.database.load_persistent_torrents().await?; let mut torrents = self.torrents.write().await; @@ -700,6 +703,55 @@ mod tests { ); } + #[tokio::test] + async fn it_should_return_all_the_peers_for_a_given_torrent() { + let tracker = public_tracker(); + + let info_hash = sample_info_hash(); + let peer = sample_peer(); + + tracker.update_torrent_with_peer_and_get_stats(&info_hash, &peer).await; + + let peers = tracker.get_all_torrent_peers(&info_hash).await; + + assert_eq!(peers, vec![peer]); + } + + #[tokio::test] + async fn it_should_return_all_the_peers_for_a_given_torrent_excluding_a_given_peer() { + let tracker = public_tracker(); + + let info_hash = sample_info_hash(); + let peer = sample_peer(); + + tracker.update_torrent_with_peer_and_get_stats(&info_hash, &peer).await; + + let peers = tracker.get_peers_for_peer(&info_hash, &peer).await; + + assert_eq!(peers, vec![]); + } + + #[tokio::test] + async fn it_should_return_the_torrent_metrics() { + let tracker = public_tracker(); + + tracker + .update_torrent_with_peer_and_get_stats(&sample_info_hash(), &leecher()) + .await; + + let torrent_metrics = tracker.get_torrents_metrics().await; + + assert_eq!( + torrent_metrics, + TorrentsMetrics { + seeders: 0, + completed: 0, + leechers: 1, + torrents: 1, + } + ); + } + mod for_all_config_modes { mod handling_an_announce_request { @@ -984,6 +1036,55 @@ mod tests { } } + mod handling_the_torrent_whitelist { + use crate::tracker::tests::the_tracker::{sample_info_hash, whitelisted_tracker}; + + #[tokio::test] + async fn it_should_add_a_torrent_to_the_whitelist() { + let tracker = whitelisted_tracker(); + + let info_hash = sample_info_hash(); + + tracker.add_torrent_to_whitelist(&info_hash).await.unwrap(); + + assert!(tracker.is_info_hash_whitelisted(&info_hash).await); + } + + #[tokio::test] + async fn it_should_remove_a_torrent_from_the_whitelist() { + let tracker = whitelisted_tracker(); + + let info_hash = sample_info_hash(); + + tracker.add_torrent_to_whitelist(&info_hash).await.unwrap(); + + tracker.remove_torrent_from_whitelist(&info_hash).await.unwrap(); + + assert!(!tracker.is_info_hash_whitelisted(&info_hash).await); + } + + mod persistence { + use crate::tracker::tests::the_tracker::{sample_info_hash, whitelisted_tracker}; + + #[tokio::test] + async fn it_should_load_the_whitelist_from_the_database() { + let tracker = whitelisted_tracker(); + + let info_hash = sample_info_hash(); + + tracker.add_torrent_to_whitelist(&info_hash).await.unwrap(); + + // Remove torrent from the in-memory whitelist + tracker.whitelist.write().await.remove(&info_hash); + assert!(!tracker.is_info_hash_whitelisted(&info_hash).await); + + tracker.load_whitelist_from_database().await.unwrap(); + + assert!(tracker.is_info_hash_whitelisted(&info_hash).await); + } + } + } + mod handling_an_announce_request {} mod handling_an_scrape_request { @@ -1112,7 +1213,7 @@ mod tests { // Remove the newly generated key in memory tracker.keys.write().await.remove(&key.id()); - let result = tracker.load_keys().await; + let result = tracker.load_keys_from_database().await; assert!(result.is_ok()); assert!(tracker.verify_auth_key(&key.id()).await.is_ok()); @@ -1152,13 +1253,10 @@ mod tests { let swarm_stats = tracker.update_torrent_with_peer_and_get_stats(&info_hash, &peer).await; assert_eq!(swarm_stats.completed, 1); - let torrents = tracker.get_all_torrent_peers(&info_hash).await; - assert_eq!(torrents.len(), 1); - // Remove the newly updated torrent from memory tracker.torrents.write().await.remove(&info_hash); - tracker.load_persistent_torrents().await.unwrap(); + tracker.load_torrents_from_database().await.unwrap(); let torrents = tracker.get_torrents().await; assert!(torrents.contains_key(&info_hash)); diff --git a/src/tracker/torrent.rs b/src/tracker/torrent.rs index 3362234f0..8675490e2 100644 --- a/src/tracker/torrent.rs +++ b/src/tracker/torrent.rs @@ -14,6 +14,7 @@ pub struct Entry { pub completed: u32, } +/// Swarm statistics for one torrent. /// Swarm metadata dictionary in the scrape response. /// BEP 48: #[derive(Debug, PartialEq, Default)] @@ -30,7 +31,8 @@ impl SwarmMetadata { } } -/// Swarm statistics. Alternative struct for swarm metadata in scrape response. +/// Swarm statistics for one torrent. +/// Alternative struct for swarm metadata in scrape response. #[derive(Debug, PartialEq, Default)] pub struct SwamStats { pub completed: u32, // The number of peers that have ever completed downloading From d0c30540e11a6dd71cefb8d4736b23adf9b87fe0 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 3 Mar 2023 17:50:27 +0000 Subject: [PATCH 12/12] fix(tracker): typo, rename SwamStats to SwarmStats --- .../axum_implementation/responses/announce.rs | 8 +++---- src/http/warp_implementation/handlers.rs | 4 ++-- src/tracker/mod.rs | 22 +++++++++---------- src/tracker/torrent.rs | 2 +- src/udp/handlers.rs | 8 +++---- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/http/axum_implementation/responses/announce.rs b/src/http/axum_implementation/responses/announce.rs index a91266490..81651767b 100644 --- a/src/http/axum_implementation/responses/announce.rs +++ b/src/http/axum_implementation/responses/announce.rs @@ -90,8 +90,8 @@ impl From for NonCompact { Self { interval: domain_announce_response.interval, interval_min: domain_announce_response.interval_min, - complete: domain_announce_response.swam_stats.seeders, - incomplete: domain_announce_response.swam_stats.leechers, + complete: domain_announce_response.swarm_stats.seeders, + incomplete: domain_announce_response.swarm_stats.leechers, peers, } } @@ -237,8 +237,8 @@ impl From for Compact { Self { interval: domain_announce_response.interval, interval_min: domain_announce_response.interval_min, - complete: domain_announce_response.swam_stats.seeders, - incomplete: domain_announce_response.swam_stats.leechers, + complete: domain_announce_response.swarm_stats.seeders, + incomplete: domain_announce_response.swarm_stats.leechers, peers, } } diff --git a/src/http/warp_implementation/handlers.rs b/src/http/warp_implementation/handlers.rs index 6019bf016..b803a594f 100644 --- a/src/http/warp_implementation/handlers.rs +++ b/src/http/warp_implementation/handlers.rs @@ -66,7 +66,7 @@ pub async fn handle_announce( send_announce_response( &announce_request, - &response.swam_stats, + &response.swarm_stats, &response.peers, tracker.config.announce_interval, tracker.config.min_announce_interval, @@ -129,7 +129,7 @@ pub async fn handle_scrape( #[allow(clippy::ptr_arg)] fn send_announce_response( announce_request: &request::Announce, - torrent_stats: &torrent::SwamStats, + torrent_stats: &torrent::SwarmStats, peers: &Vec, interval: u32, interval_min: u32, diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 3048ded35..fbda95354 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -19,7 +19,7 @@ use tokio::sync::{RwLock, RwLockReadGuard}; use self::auth::KeyId; use self::error::Error; use self::peer::Peer; -use self::torrent::{SwamStats, SwarmMetadata}; +use self::torrent::{SwarmMetadata, SwarmStats}; use crate::config::Configuration; use crate::databases::driver::Driver; use crate::databases::{self, Database}; @@ -38,9 +38,9 @@ pub struct Tracker { #[derive(Debug, PartialEq, Default)] pub struct TorrentsMetrics { - // code-review: consider using `SwamStats` for + // code-review: consider using `SwarmStats` for // `seeders`, `completed`, and `leechers` attributes. - // pub swam_stats: SwamStats; + // pub swarm_stats: SwarmStats; pub seeders: u64, pub completed: u64, pub leechers: u64, @@ -50,7 +50,7 @@ pub struct TorrentsMetrics { #[derive(Debug, PartialEq, Default)] pub struct AnnounceData { pub peers: Vec, - pub swam_stats: SwamStats, + pub swarm_stats: SwarmStats, pub interval: u32, pub interval_min: u32, } @@ -147,13 +147,13 @@ impl Tracker { peer.change_ip(&assign_ip_address_to_peer(remote_client_ip, self.config.get_ext_ip())); - let swam_stats = self.update_torrent_with_peer_and_get_stats(info_hash, peer).await; + let swarm_stats = self.update_torrent_with_peer_and_get_stats(info_hash, peer).await; let peers = self.get_peers_for_peer(info_hash, peer).await; AnnounceData { peers, - swam_stats, + swarm_stats, interval: self.config.announce_interval, interval_min: self.config.min_announce_interval, } @@ -446,7 +446,7 @@ impl Tracker { } } - pub async fn update_torrent_with_peer_and_get_stats(&self, info_hash: &InfoHash, peer: &peer::Peer) -> torrent::SwamStats { + pub async fn update_torrent_with_peer_and_get_stats(&self, info_hash: &InfoHash, peer: &peer::Peer) -> torrent::SwarmStats { // code-review: consider splitting the function in two (command and query segregation). // `update_torrent_with_peer` and `get_stats` @@ -469,7 +469,7 @@ impl Tracker { let (seeders, completed, leechers) = torrent_entry.get_stats(); - torrent::SwamStats { + torrent::SwarmStats { completed, seeders, leechers, @@ -898,7 +898,7 @@ mod tests { let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; - assert_eq!(announce_data.swam_stats.seeders, 1); + assert_eq!(announce_data.swarm_stats.seeders, 1); } #[tokio::test] @@ -909,7 +909,7 @@ mod tests { let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()).await; - assert_eq!(announce_data.swam_stats.leechers, 1); + assert_eq!(announce_data.swarm_stats.leechers, 1); } #[tokio::test] @@ -923,7 +923,7 @@ mod tests { let mut completed_peer = completed_peer(); let announce_data = tracker.announce(&sample_info_hash(), &mut completed_peer, &peer_ip()).await; - assert_eq!(announce_data.swam_stats.completed, 1); + assert_eq!(announce_data.swarm_stats.completed, 1); } } } diff --git a/src/tracker/torrent.rs b/src/tracker/torrent.rs index 8675490e2..4a871aa89 100644 --- a/src/tracker/torrent.rs +++ b/src/tracker/torrent.rs @@ -34,7 +34,7 @@ impl SwarmMetadata { /// Swarm statistics for one torrent. /// Alternative struct for swarm metadata in scrape response. #[derive(Debug, PartialEq, Default)] -pub struct SwamStats { +pub struct SwarmStats { pub completed: u32, // The number of peers that have ever completed downloading pub seeders: u32, // The number of active peers that have completed downloading (seeders) pub leechers: u32, // The number of active peers that have not completed downloading (leechers) diff --git a/src/udp/handlers.rs b/src/udp/handlers.rs index 6c54a6106..8fda77fb4 100644 --- a/src/udp/handlers.rs +++ b/src/udp/handlers.rs @@ -136,8 +136,8 @@ pub async fn handle_announce( Response::from(AnnounceResponse { transaction_id: wrapped_announce_request.announce_request.transaction_id, announce_interval: AnnounceInterval(i64::from(tracker.config.announce_interval) as i32), - leechers: NumberOfPeers(i64::from(response.swam_stats.leechers) as i32), - seeders: NumberOfPeers(i64::from(response.swam_stats.seeders) as i32), + leechers: NumberOfPeers(i64::from(response.swarm_stats.leechers) as i32), + seeders: NumberOfPeers(i64::from(response.swarm_stats.seeders) as i32), peers: response .peers .iter() @@ -157,8 +157,8 @@ pub async fn handle_announce( Response::from(AnnounceResponse { transaction_id: wrapped_announce_request.announce_request.transaction_id, announce_interval: AnnounceInterval(i64::from(tracker.config.announce_interval) as i32), - leechers: NumberOfPeers(i64::from(response.swam_stats.leechers) as i32), - seeders: NumberOfPeers(i64::from(response.swam_stats.seeders) as i32), + leechers: NumberOfPeers(i64::from(response.swarm_stats.leechers) as i32), + seeders: NumberOfPeers(i64::from(response.swarm_stats.seeders) as i32), peers: response .peers .iter()