Skip to content

Commit

Permalink
refactor: [#1182] remove whitelist context methods from core tracker
Browse files Browse the repository at this point in the history
  • Loading branch information
@josecelano
josecelano committed Jan 15, 2025
1 parent 882af33 commit 57455ca
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 76 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
95 changes: 29 additions & 66 deletions src/core/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1044,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 @@ -1054,48 +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.whitelist_manager.add_torrent_to_whitelist(info_hash).await
}

/// 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.whitelist_manager.remove_torrent_from_whitelist(info_hash).await
}

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

/// 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> {
self.whitelist_manager.load_whitelist_from_database().await
}

/// It return the `Tracker` [`statistics::metrics::Metrics`].
///
/// # Context: Statistics
Expand Down Expand Up @@ -1156,6 +1114,7 @@ mod tests {
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 {
Expand All @@ -1170,10 +1129,12 @@ mod tests {
tracker_factory(&config, &database, &whitelist_manager)
}

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

(tracker, whitelist_manager)
}

pub fn tracker_persisting_torrents_in_database() -> Tracker {
Expand Down Expand Up @@ -1707,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 @@ -1720,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 @@ -1732,51 +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_manager
.remove_torrent_from_memory_whitelist(&info_hash)
.await;
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 @@ -1807,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
8 changes: 8 additions & 0 deletions tests/servers/api/environment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use torrust_tracker_api_client::connection_info::{ConnectionInfo, Origin};
use torrust_tracker_configuration::{Configuration, HttpApi};
use torrust_tracker_lib::bootstrap::app::initialize_with_configuration;
use torrust_tracker_lib::bootstrap::jobs::make_rust_tls;
use torrust_tracker_lib::core::whitelist::WhiteListManager;
use torrust_tracker_lib::core::Tracker;
use torrust_tracker_lib::servers::apis::server::{ApiServer, Launcher, Running, Stopped};
use torrust_tracker_lib::servers::registar::Registar;
Expand All @@ -21,6 +22,7 @@ where
{
pub config: Arc<HttpApi>,
pub tracker: Arc<Tracker>,
pub whitelist_manager: Arc<WhiteListManager>,
pub ban_service: Arc<RwLock<BanService>>,
pub registar: Registar,
pub server: ApiServer<S>,
Expand All @@ -40,6 +42,9 @@ impl Environment<Stopped> {
pub fn new(configuration: &Arc<Configuration>) -> Self {
let tracker = initialize_with_configuration(configuration);

// todo: get from `initialize_with_configuration`
let whitelist_manager = tracker.whitelist_manager.clone();

let ban_service = Arc::new(RwLock::new(BanService::new(MAX_CONNECTION_ID_ERRORS_PER_IP)));

let config = Arc::new(configuration.http_api.clone().expect("missing API configuration"));
Expand All @@ -53,6 +58,7 @@ impl Environment<Stopped> {
Self {
config,
tracker,
whitelist_manager,
ban_service,
registar: Registar::default(),
server,
Expand All @@ -65,6 +71,7 @@ impl Environment<Stopped> {
Environment {
config: self.config,
tracker: self.tracker.clone(),
whitelist_manager: self.whitelist_manager.clone(),
ban_service: self.ban_service.clone(),
registar: self.registar.clone(),
server: self
Expand All @@ -85,6 +92,7 @@ impl Environment<Running> {
Environment {
config: self.config,
tracker: self.tracker,
whitelist_manager: self.whitelist_manager,
ban_service: self.ban_service,
registar: Registar::default(),
server: self.server.stop().await.unwrap(),
Expand Down
16 changes: 8 additions & 8 deletions tests/servers/api/v1/contract/context/whitelist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async fn should_allow_whitelisting_a_torrent() {

assert_ok(response).await;
assert!(
env.tracker
env.whitelist_manager
.is_info_hash_whitelisted(&InfoHash::from_str(&info_hash).unwrap())
.await
);
Expand Down Expand Up @@ -167,7 +167,7 @@ async fn should_allow_removing_a_torrent_from_the_whitelist() {
let hash = "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d".to_owned();
let info_hash = InfoHash::from_str(&hash).unwrap();

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

let request_id = Uuid::new_v4();

Expand All @@ -176,7 +176,7 @@ async fn should_allow_removing_a_torrent_from_the_whitelist() {
.await;

assert_ok(response).await;
assert!(!env.tracker.is_info_hash_whitelisted(&info_hash).await);
assert!(!env.whitelist_manager.is_info_hash_whitelisted(&info_hash).await);

env.stop().await;
}
Expand Down Expand Up @@ -237,7 +237,7 @@ async fn should_fail_when_the_torrent_cannot_be_removed_from_the_whitelist() {

let hash = "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d".to_owned();
let info_hash = InfoHash::from_str(&hash).unwrap();
env.tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
env.whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

force_database_error(&env.tracker);

Expand Down Expand Up @@ -266,7 +266,7 @@ async fn should_not_allow_removing_a_torrent_from_the_whitelist_for_unauthentica
let hash = "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d".to_owned();
let info_hash = InfoHash::from_str(&hash).unwrap();

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

let request_id = Uuid::new_v4();

Expand All @@ -281,7 +281,7 @@ async fn should_not_allow_removing_a_torrent_from_the_whitelist_for_unauthentica
"Expected logs to contain: ERROR ... API ... request_id={request_id}"
);

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

let request_id = Uuid::new_v4();

Expand All @@ -307,7 +307,7 @@ async fn should_allow_reload_the_whitelist_from_the_database() {

let hash = "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d".to_owned();
let info_hash = InfoHash::from_str(&hash).unwrap();
env.tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
env.whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

let request_id = Uuid::new_v4();

Expand Down Expand Up @@ -338,7 +338,7 @@ async fn should_fail_when_the_whitelist_cannot_be_reloaded_from_the_database() {

let hash = "9e0217d0fa71c87332cd8bf9dbeabcb2c2cf3c4d".to_owned();
let info_hash = InfoHash::from_str(&hash).unwrap();
env.tracker.add_torrent_to_whitelist(&info_hash).await.unwrap();
env.whitelist_manager.add_torrent_to_whitelist(&info_hash).await.unwrap();

force_database_error(&env.tracker);

Expand Down
7 changes: 7 additions & 0 deletions tests/servers/http/environment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use futures::executor::block_on;
use torrust_tracker_configuration::{Configuration, HttpTracker};
use torrust_tracker_lib::bootstrap::app::initialize_with_configuration;
use torrust_tracker_lib::bootstrap::jobs::make_rust_tls;
use torrust_tracker_lib::core::whitelist::WhiteListManager;
use torrust_tracker_lib::core::Tracker;
use torrust_tracker_lib::servers::http::server::{HttpServer, Launcher, Running, Stopped};
use torrust_tracker_lib::servers::registar::Registar;
Expand All @@ -13,6 +14,7 @@ use torrust_tracker_primitives::peer;
pub struct Environment<S> {
pub config: Arc<HttpTracker>,
pub tracker: Arc<Tracker>,
pub whitelist_manager: Arc<WhiteListManager>,
pub registar: Registar,
pub server: HttpServer<S>,
}
Expand All @@ -29,6 +31,8 @@ impl Environment<Stopped> {
pub fn new(configuration: &Arc<Configuration>) -> Self {
let tracker = initialize_with_configuration(configuration);

let whitelist_manager = tracker.whitelist_manager.clone();

let http_tracker = configuration
.http_trackers
.clone()
Expand All @@ -45,6 +49,7 @@ impl Environment<Stopped> {
Self {
config,
tracker,
whitelist_manager,
registar: Registar::default(),
server,
}
Expand All @@ -55,6 +60,7 @@ impl Environment<Stopped> {
Environment {
config: self.config,
tracker: self.tracker.clone(),
whitelist_manager: self.whitelist_manager.clone(),
registar: self.registar.clone(),
server: self.server.start(self.tracker, self.registar.give_form()).await.unwrap(),
}
Expand All @@ -70,6 +76,7 @@ impl Environment<Running> {
Environment {
config: self.config,
tracker: self.tracker,
whitelist_manager: self.whitelist_manager,
registar: Registar::default(),

server: self.server.stop().await.unwrap(),
Expand Down
4 changes: 2 additions & 2 deletions tests/servers/http/v1/contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,7 @@ mod configured_as_whitelisted {

let info_hash = InfoHash::from_str("9c38422213e30bff212b30c360d26f9a02136422").unwrap();

env.tracker
env.whitelist_manager
.add_torrent_to_whitelist(&info_hash)
.await
.expect("should add the torrent to the whitelist");
Expand Down Expand Up @@ -1343,7 +1343,7 @@ mod configured_as_whitelisted {
.build(),
);

env.tracker
env.whitelist_manager
.add_torrent_to_whitelist(&info_hash)
.await
.expect("should add the torrent to the whitelist");
Expand Down

0 comments on commit 57455ca

Please sign in to comment.