From adee3b5267386b642b7b09519eebf049a44f5eea Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 19 Oct 2022 11:03:05 +0100 Subject: [PATCH 1/2] fix: [#97] make tracker statistics optional again Commit 7abe0f5bde1e209553d1a1e2d6fe644cd46a9395 introduced an unwanted change. Thread for statistics is always created regardless configuration. This commit reverts that change. The config option: config.tracker_usage_statistics defines wether the statistics should be enabled or not. --- src/main.rs | 6 +++++- src/tracker/statistics.rs | 6 ------ src/udp/handlers.rs | 18 +++++++++++------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index bac7854bb..ffe080f9a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,7 +24,11 @@ async fn main() { }; // Initialize stats tracker - let stats_tracker = StatsTracker::new_running_instance(); + let mut stats_tracker = StatsTracker::new(); + + if config.tracker_usage_statistics { + stats_tracker.run_worker(); + } // Initialize Torrust tracker let tracker = match TorrentTracker::new(config.clone(), Box::new(stats_tracker)) { diff --git a/src/tracker/statistics.rs b/src/tracker/statistics.rs index a2a0de99b..2a216770e 100644 --- a/src/tracker/statistics.rs +++ b/src/tracker/statistics.rs @@ -62,12 +62,6 @@ pub struct StatsTracker { } impl StatsTracker { - pub fn new_running_instance() -> Self { - let mut stats_tracker = Self::new(); - stats_tracker.run_worker(); - stats_tracker - } - pub fn new() -> Self { Self { channel_sender: None, diff --git a/src/udp/handlers.rs b/src/udp/handlers.rs index d46cd9231..8117b6c89 100644 --- a/src/udp/handlers.rs +++ b/src/udp/handlers.rs @@ -271,17 +271,23 @@ mod tests { fn initialized_public_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Public).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_running_instance())).unwrap()) + Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) } fn initialized_private_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Private).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_running_instance())).unwrap()) + Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) } fn initialized_whitelisted_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Listed).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_running_instance())).unwrap()) + Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) + } + + fn initialized_stats_tracker() -> StatsTracker { + let mut stats_tracker = StatsTracker::new(); + stats_tracker.run_worker(); + stats_tracker } fn sample_ipv4_remote_addr() -> SocketAddr { @@ -959,18 +965,16 @@ mod tests { use aquatic_udp_protocol::{InfoHash as AquaticInfoHash, PeerId as AquaticPeerId}; - use crate::statistics::StatsTracker; use crate::tracker::tracker::TorrentTracker; use crate::udp::connection_cookie::{into_connection_id, make_connection_cookie}; use crate::udp::handle_announce; use crate::udp::handlers::tests::announce_request::AnnounceRequestBuilder; - use crate::udp::handlers::tests::TrackerConfigurationBuilder; + use crate::udp::handlers::tests::{initialized_stats_tracker, TrackerConfigurationBuilder}; #[tokio::test] async fn the_peer_ip_should_be_changed_to_the_external_ip_in_the_tracker_configuration() { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_external_ip("::126.0.0.1").into()); - let tracker = - Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_running_instance())).unwrap()); + let tracker = Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()); let loopback_ipv4 = Ipv4Addr::new(127, 0, 0, 1); let loopback_ipv6 = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1); From e12d8e6ec142883ae5734113920f7c8cdb80c30a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 19 Oct 2022 18:29:13 +0100 Subject: [PATCH 2/2] test: [#97] add test for optional statistics Tracker statistics can be enabled or disabled using the configuration option `tracker_usage_statistics`. This commit adds tests for that behavior. --- src/main.rs | 6 +---- src/tracker/statistics.rs | 51 +++++++++++++++++++++++++++++++++++++++ src/udp/handlers.rs | 18 ++++++-------- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/main.rs b/src/main.rs index ffe080f9a..dcb92acb8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,11 +24,7 @@ async fn main() { }; // Initialize stats tracker - let mut stats_tracker = StatsTracker::new(); - - if config.tracker_usage_statistics { - stats_tracker.run_worker(); - } + let stats_tracker = StatsTracker::new_instance(config.tracker_usage_statistics); // Initialize Torrust tracker let tracker = match TorrentTracker::new(config.clone(), Box::new(stats_tracker)) { diff --git a/src/tracker/statistics.rs b/src/tracker/statistics.rs index 2a216770e..fb4e4c0fe 100644 --- a/src/tracker/statistics.rs +++ b/src/tracker/statistics.rs @@ -62,6 +62,27 @@ pub struct StatsTracker { } impl StatsTracker { + pub fn new_active_instance() -> Self { + Self::new_instance(true) + } + + pub fn new_inactive_instance() -> Self { + Self::new_instance(false) + } + + pub fn new_instance(active: bool) -> Self { + let mut stats_tracker = Self { + channel_sender: None, + stats: Arc::new(RwLock::new(TrackerStatistics::new())), + }; + + if active { + stats_tracker.run_worker(); + } + + stats_tracker + } + pub fn new() -> Self { Self { channel_sender: None, @@ -155,3 +176,33 @@ impl TrackerStatisticsRepository for StatsTracker { pub trait TrackerStatsService: TrackerStatisticsEventSender + TrackerStatisticsRepository {} impl TrackerStatsService for StatsTracker {} + +#[cfg(test)] +mod test { + + mod event_sender { + use crate::statistics::{StatsTracker, TrackerStatisticsEvent, TrackerStatisticsEventSender}; + + #[tokio::test] + async fn should_not_send_any_event_when_statistics_are_disabled() { + let tracker_usage_statistics = false; + + let inactive_stats_tracker = StatsTracker::new_instance(tracker_usage_statistics); + + let result = inactive_stats_tracker.send_event(TrackerStatisticsEvent::Tcp4Announce).await; + + assert!(result.is_none()); + } + + #[tokio::test] + async fn should_send_events_when_statistics_are_enabled() { + let tracker_usage_statistics = true; + + let active_stats_tracker = StatsTracker::new_instance(tracker_usage_statistics); + + let result = active_stats_tracker.send_event(TrackerStatisticsEvent::Tcp4Announce).await; + + assert!(result.is_some()); + } + } +} diff --git a/src/udp/handlers.rs b/src/udp/handlers.rs index 8117b6c89..845b860e9 100644 --- a/src/udp/handlers.rs +++ b/src/udp/handlers.rs @@ -271,23 +271,17 @@ mod tests { fn initialized_public_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Public).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) + Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_active_instance())).unwrap()) } fn initialized_private_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Private).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) + Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_active_instance())).unwrap()) } fn initialized_whitelisted_tracker() -> Arc { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_mode(TrackerMode::Listed).into()); - Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()) - } - - fn initialized_stats_tracker() -> StatsTracker { - let mut stats_tracker = StatsTracker::new(); - stats_tracker.run_worker(); - stats_tracker + Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_active_instance())).unwrap()) } fn sample_ipv4_remote_addr() -> SocketAddr { @@ -965,16 +959,18 @@ mod tests { use aquatic_udp_protocol::{InfoHash as AquaticInfoHash, PeerId as AquaticPeerId}; + use crate::statistics::StatsTracker; use crate::tracker::tracker::TorrentTracker; use crate::udp::connection_cookie::{into_connection_id, make_connection_cookie}; use crate::udp::handle_announce; use crate::udp::handlers::tests::announce_request::AnnounceRequestBuilder; - use crate::udp::handlers::tests::{initialized_stats_tracker, TrackerConfigurationBuilder}; + use crate::udp::handlers::tests::TrackerConfigurationBuilder; #[tokio::test] async fn the_peer_ip_should_be_changed_to_the_external_ip_in_the_tracker_configuration() { let configuration = Arc::new(TrackerConfigurationBuilder::default().with_external_ip("::126.0.0.1").into()); - let tracker = Arc::new(TorrentTracker::new(configuration, Box::new(initialized_stats_tracker())).unwrap()); + let tracker = + Arc::new(TorrentTracker::new(configuration, Box::new(StatsTracker::new_active_instance())).unwrap()); let loopback_ipv4 = Ipv4Addr::new(127, 0, 0, 1); let loopback_ipv6 = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1);