Skip to content

Commit

Permalink
Merge torrust#1183: Refactor: extract WhitelistManager
Browse files Browse the repository at this point in the history 
57455ca refactor: [torrust#1182] remove whitelist context methods from core tracker (Jose Celano)
882af33 refactor: [torrust#1182] remove duplicate code (Jose Celano)
658d2be refactor: [torrust#1182] inject database and whitelist manager in tracker factory (Jose Celano)
4253d0f refactor: [torrust#1182] use WhitelistManager in API handlers (Jose Celano)
2f1abeb refactor: [torrust#1182] inject database and whitelist in tracker as dep (Jose Celano)
cc2bc7b refactor: [torrust#1182] move structs to new mods in whitelist (Jose Celano)
07f53a4 refactor: [torrust#1182] extract strcut DatabaseWhitelist (Jose Celano)
ea35ba5 refactor: [torrust#1182] extract struct InMemoryWhitelist (Jose Celano)
39d1620 refactor: [torrust#1182] remove unused methods (Jose Celano)
439493d refactor: [torrust#1182] move extracted service to new mod (Jose Celano)
40eb805 refactor: [1182] extract WhitelistManager (Jose Celano)

Pull request description:

  Refactor: extract `WhitelistManager`

ACKs for top commit:
  josecelano:
    ACK 57455ca

Tree-SHA512: 03fb1b590b8226b96f178b80ce8998955dc70892cba1b9f921138e29cfde5e51df71d3956966338f009b12915b33bbfbb1ff1514c24e8f7868eda402d2918a73
  • Loading branch information
@josecelano
josecelano committed Jan 15, 2025
2 parents b403382 + 57455ca commit b7e009d
Show file tree
Hide file tree
Showing 21 changed files with 541 additions and 307 deletions.
1 change: 1 addition & 0 deletions src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub async fn start(
// Load whitelisted torrents
if tracker.is_listed() {
tracker
.whitelist_manager
.load_whitelist_from_database()
.await
.expect("Could not load whitelist from database.");
Expand Down
16 changes: 14 additions & 2 deletions src/bootstrap/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use tracing::instrument;

use super::config::initialize_configuration;
use crate::bootstrap;
use crate::core::services::tracker_factory;
use crate::core::databases::Database;
use crate::core::services::{initialize_database, initialize_whitelist, tracker_factory};
use crate::core::whitelist::WhiteListManager;
use crate::core::Tracker;
use crate::servers::udp::server::banning::BanService;
use crate::servers::udp::server::launcher::MAX_CONNECTION_ID_ERRORS_PER_IP;
Expand Down Expand Up @@ -105,7 +107,17 @@ pub fn initialize_static() {
#[must_use]
#[instrument(skip(config))]
pub fn initialize_tracker(config: &Configuration) -> Tracker {
tracker_factory(config)
let (database, whitelist_manager) = initialize_tracker_dependencies(config);

tracker_factory(config, &database, &whitelist_manager)
}

#[must_use]
pub fn initialize_tracker_dependencies(config: &Configuration) -> (Arc<Box<dyn Database>>, Arc<WhiteListManager>) {
let database = initialize_database(config);
let whitelist_manager = initialize_whitelist(database.clone());

(database, whitelist_manager)
}

/// It initializes the log threshold, format and channel.
Expand Down
185 changes: 49 additions & 136 deletions src/core/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ pub mod error;
pub mod services;
pub mod statistics;
pub mod torrent;
pub mod whitelist;

pub mod peer_tests;

Expand All @@ -456,11 +457,9 @@ use std::time::Duration;

use auth::PeerKey;
use bittorrent_primitives::info_hash::InfoHash;
use databases::driver::Driver;
use error::PeerKeyError;
use tokio::sync::mpsc::error::SendError;
use torrust_tracker_clock::clock::Time;
use torrust_tracker_configuration::v2_0_0::database;
use torrust_tracker_configuration::{AnnouncePolicy, Core, TORRENT_PEERS_LIMIT};
use torrust_tracker_located_error::Located;
use torrust_tracker_primitives::core::{AnnounceData, ScrapeData};
Expand All @@ -470,6 +469,7 @@ use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch};
use torrust_tracker_torrent_repository::entry::EntrySync;
use torrust_tracker_torrent_repository::repository::Repository;
use tracing::instrument;
use whitelist::WhiteListManager;

use self::auth::Key;
use self::error::Error;
Expand Down Expand Up @@ -498,7 +498,7 @@ pub struct Tracker {
keys: tokio::sync::RwLock<std::collections::HashMap<Key, auth::PeerKey>>,

/// The list of allowed torrents. Only for listed trackers.
whitelist: tokio::sync::RwLock<std::collections::HashSet<InfoHash>>,
pub whitelist_manager: Arc<WhiteListManager>,

/// The in-memory torrents repository.
torrents: Arc<Torrents>,
Expand Down Expand Up @@ -574,24 +574,19 @@ impl Tracker {
/// Will return a `databases::error::Error` if unable to connect to database. The `Tracker` is responsible for the persistence.
pub fn new(
config: &Core,
database: &Arc<Box<dyn Database>>,
whitelist_manager: &Arc<WhiteListManager>,
stats_event_sender: Option<Box<dyn statistics::event::sender::Sender>>,
stats_repository: statistics::repository::Repository,
) -> Result<Tracker, databases::error::Error> {
let driver = match config.database.driver {
database::Driver::Sqlite3 => Driver::Sqlite3,
database::Driver::MySQL => Driver::MySQL,
};

let database = Arc::new(databases::driver::build(&driver, &config.database.path)?);

Ok(Tracker {
config: config.clone(),
database: database.clone(),
keys: tokio::sync::RwLock::new(std::collections::HashMap::new()),
whitelist: tokio::sync::RwLock::new(std::collections::HashSet::new()),
whitelist_manager: whitelist_manager.clone(),
torrents: Arc::default(),
stats_event_sender,
stats_repository,
database,
})
}

Expand Down Expand Up @@ -1049,7 +1044,7 @@ impl Tracker {
return Ok(());
}

if self.is_info_hash_whitelisted(info_hash).await {
if self.whitelist_manager.is_info_hash_whitelisted(info_hash).await {
return Ok(());
}

Expand All @@ -1059,104 +1054,6 @@ impl Tracker {
})
}

/// It adds a torrent to the whitelist.
/// Adding torrents is not relevant to public trackers.
///
/// # Context: Whitelist
///
/// # Errors
///
/// Will return a `database::Error` if unable to add the `info_hash` into the whitelist database.
pub async fn add_torrent_to_whitelist(&self, info_hash: &InfoHash) -> Result<(), databases::error::Error> {
self.add_torrent_to_database_whitelist(info_hash)?;
self.add_torrent_to_memory_whitelist(info_hash).await;
Ok(())
}

/// It adds a torrent to the whitelist if it has not been whitelisted previously
fn add_torrent_to_database_whitelist(&self, info_hash: &InfoHash) -> Result<(), databases::error::Error> {
let is_whitelisted = self.database.is_info_hash_whitelisted(*info_hash)?;

if is_whitelisted {
return Ok(());
}

self.database.add_info_hash_to_whitelist(*info_hash)?;

Ok(())
}

pub async fn add_torrent_to_memory_whitelist(&self, info_hash: &InfoHash) -> bool {
self.whitelist.write().await.insert(*info_hash)
}

/// It removes a torrent from the whitelist.
/// Removing torrents is not relevant to public trackers.
///
/// # Context: Whitelist
///
/// # Errors
///
/// Will return a `database::Error` if unable to remove the `info_hash` from the whitelist database.
pub async fn remove_torrent_from_whitelist(&self, info_hash: &InfoHash) -> Result<(), databases::error::Error> {
self.remove_torrent_from_database_whitelist(info_hash)?;
self.remove_torrent_from_memory_whitelist(info_hash).await;
Ok(())
}

/// It removes a torrent from the whitelist in the database.
///
/// # Context: Whitelist
///
/// # Errors
///
/// Will return a `database::Error` if unable to remove the `info_hash` from the whitelist database.
pub fn remove_torrent_from_database_whitelist(&self, info_hash: &InfoHash) -> Result<(), databases::error::Error> {
let is_whitelisted = self.database.is_info_hash_whitelisted(*info_hash)?;

if !is_whitelisted {
return Ok(());
}

self.database.remove_info_hash_from_whitelist(*info_hash)?;

Ok(())
}

/// It removes a torrent from the whitelist in memory.
///
/// # Context: Whitelist
pub async fn remove_torrent_from_memory_whitelist(&self, info_hash: &InfoHash) -> bool {
self.whitelist.write().await.remove(info_hash)
}

/// It checks if a torrent is whitelisted.
///
/// # Context: Whitelist
pub async fn is_info_hash_whitelisted(&self, info_hash: &InfoHash) -> bool {
self.whitelist.read().await.contains(info_hash)
}

/// It loads the whitelist from the database.
///
/// # Context: Whitelist
///
/// # Errors
///
/// Will return a `database::Error` if unable to load the list whitelisted `info_hash`s from the database.
pub async fn load_whitelist_from_database(&self) -> Result<(), databases::error::Error> {
let whitelisted_torrents_from_database = self.database.load_whitelist()?;
let mut whitelist = self.whitelist.write().await;

whitelist.clear();

for info_hash in whitelisted_torrents_from_database {
let _: bool = whitelist.insert(info_hash);
}

Ok(())
}

/// It return the `Tracker` [`statistics::metrics::Metrics`].
///
/// # Context: Statistics
Expand Down Expand Up @@ -1214,26 +1111,37 @@ mod tests {
use torrust_tracker_primitives::DurationSinceUnixEpoch;
use torrust_tracker_test_helpers::configuration;

use crate::bootstrap::app::initialize_tracker_dependencies;
use crate::core::peer::Peer;
use crate::core::services::tracker_factory;
use crate::core::whitelist::WhiteListManager;
use crate::core::{TorrentsMetrics, Tracker};

fn public_tracker() -> Tracker {
tracker_factory(&configuration::ephemeral_public())
let config = configuration::ephemeral_public();
let (database, whitelist_manager) = initialize_tracker_dependencies(&config);
tracker_factory(&config, &database, &whitelist_manager)
}

fn private_tracker() -> Tracker {
tracker_factory(&configuration::ephemeral_private())
let config = configuration::ephemeral_private();
let (database, whitelist_manager) = initialize_tracker_dependencies(&config);
tracker_factory(&config, &database, &whitelist_manager)
}

fn whitelisted_tracker() -> Tracker {
tracker_factory(&configuration::ephemeral_listed())
fn whitelisted_tracker() -> (Tracker, Arc<WhiteListManager>) {
let config = configuration::ephemeral_listed();
let (database, whitelist_manager) = initialize_tracker_dependencies(&config);
let tracker = tracker_factory(&config, &database, &whitelist_manager);

(tracker, whitelist_manager)
}

pub fn tracker_persisting_torrents_in_database() -> Tracker {
let mut configuration = configuration::ephemeral();
configuration.core.tracker_policy.persistent_torrent_completed_stat = true;
tracker_factory(&configuration)
let mut config = configuration::ephemeral_listed();
config.core.tracker_policy.persistent_torrent_completed_stat = true;
let (database, whitelist_manager) = initialize_tracker_dependencies(&config);
tracker_factory(&config, &database, &whitelist_manager)
}

fn sample_info_hash() -> InfoHash {
Expand Down Expand Up @@ -1760,11 +1668,11 @@ mod tests {

#[tokio::test]
async fn it_should_authorize_the_announce_and_scrape_actions_on_whitelisted_torrents() {
let tracker = whitelisted_tracker();
let (tracker, whitelist_manager) = whitelisted_tracker();

let info_hash = sample_info_hash();

let result = tracker.add_torrent_to_whitelist(&info_hash).await;
let result = whitelist_manager.add_torrent_to_whitelist(&info_hash).await;
assert!(result.is_ok());

let result = tracker.authorize(&info_hash).await;
Expand All @@ -1773,7 +1681,7 @@ mod tests {

#[tokio::test]
async fn it_should_not_authorize_the_announce_and_scrape_actions_on_not_whitelisted_torrents() {
let tracker = whitelisted_tracker();
let (tracker, _whitelist_manager) = whitelisted_tracker();

let info_hash = sample_info_hash();

Expand All @@ -1785,48 +1693,53 @@ mod tests {
mod handling_the_torrent_whitelist {
use crate::core::tests::the_tracker::{sample_info_hash, whitelisted_tracker};

// todo: after extracting the WhitelistManager from the Tracker,
// there is no need to use the tracker to test the whitelist.
// Test not using the `tracker` (`_tracker` variable) should be
// moved to the whitelist module.

#[tokio::test]
async fn it_should_add_a_torrent_to_the_whitelist() {
let tracker = whitelisted_tracker();
let (_tracker, whitelist_manager) = whitelisted_tracker();

let info_hash = sample_info_hash();

tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

assert!(tracker.is_info_hash_whitelisted(&info_hash).await);
assert!(whitelist_manager.is_info_hash_whitelisted(&info_hash).await);
}

#[tokio::test]
async fn it_should_remove_a_torrent_from_the_whitelist() {
let tracker = whitelisted_tracker();
let (_tracker, whitelist_manager) = whitelisted_tracker();

let info_hash = sample_info_hash();

tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

tracker.remove_torrent_from_whitelist(&info_hash).await.unwrap();
whitelist_manager.remove_torrent_from_whitelist(&info_hash).await.unwrap();

assert!(!tracker.is_info_hash_whitelisted(&info_hash).await);
assert!(!whitelist_manager.is_info_hash_whitelisted(&info_hash).await);
}

mod persistence {
use crate::core::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 (_tracker, whitelist_manager) = whitelisted_tracker();

let info_hash = sample_info_hash();

tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

whitelist_manager.remove_torrent_from_memory_whitelist(&info_hash).await;

// Remove torrent from the in-memory whitelist
tracker.whitelist.write().await.remove(&info_hash);
assert!(!tracker.is_info_hash_whitelisted(&info_hash).await);
assert!(!whitelist_manager.is_info_hash_whitelisted(&info_hash).await);

tracker.load_whitelist_from_database().await.unwrap();
whitelist_manager.load_whitelist_from_database().await.unwrap();

assert!(tracker.is_info_hash_whitelisted(&info_hash).await);
assert!(whitelist_manager.is_info_hash_whitelisted(&info_hash).await);
}
}
}
Expand Down Expand Up @@ -1857,7 +1770,7 @@ mod tests {

#[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 (tracker, _whitelist_manager) = whitelisted_tracker();

let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::<InfoHash>().unwrap();

Expand Down
Loading

0 comments on commit b7e009d

Please sign in to comment.