Skip to content

Commit

Permalink
Merge #1252: Overhaul core Tracker: add tests for error mod
Browse files Browse the repository at this point in the history
ca4ead5 test: [#1250] add tests for tracker core errors (Jose Celano)
0811d67 refactor: [#1250] rename enum (Jose Celano)
3bb4a13 refactor: [#1250] remove unused errors in tracker core (Jose Celano)
58a3741 refactor: [#1250] invert dependency between http-protocol and tracker-core pacakges (Jose Celano)

Pull request description:

  Overhaul core Tracker: add tests for `error` mod.

ACKs for top commit:
  josecelano:
    ACK ca4ead5

Tree-SHA512: 0d1b1c3cda7595509350552c1edc143bf729c1bb9f05673b4d8707e1fa964473739bd8d8b3b275e16ced68fad27275162bd985b20dfc95e62ac18bf09cdf92b2
  • Loading branch information
josecelano committed Feb 7, 2025
2 parents a1046a2 + ca4ead5 commit c1586de
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 34 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/http-protocol/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ version.workspace = true
[dependencies]
aquatic_udp_protocol = "0"
bittorrent-primitives = "0.1.0"
bittorrent-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" }
derive_more = { version = "1", features = ["as_ref", "constructor", "from"] }
multimap = "0"
percent-encoding = "2"
Expand Down
8 changes: 8 additions & 0 deletions packages/http-protocol/src/v1/responses/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ impl From<PeerIpResolutionError> for Error {
}
}

impl From<bittorrent_tracker_core::error::WhitelistError> for Error {
fn from(err: bittorrent_tracker_core::error::WhitelistError) -> Self {
Error {
failure_reason: format!("Tracker error: {err}"),
}
}
}

#[cfg(test)]
mod tests {

Expand Down
1 change: 0 additions & 1 deletion packages/tracker-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ version.workspace = true

[dependencies]
aquatic_udp_protocol = "0"
bittorrent-http-protocol = { version = "3.0.0-develop", path = "../http-protocol" }
bittorrent-primitives = "0.1.0"
chrono = { version = "0", default-features = false, features = ["clock"] }
derive_more = { version = "1", features = ["as_ref", "constructor", "from"] }
Expand Down
110 changes: 83 additions & 27 deletions packages/tracker-core/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,23 @@
//! Error returned by the core `Tracker`.
//!
//! Error | Context | Description
//! ---|---|---
//! `PeerKeyNotValid` | Authentication | The supplied key is not valid. It may not be registered or expired.
//! `PeerNotAuthenticated` | Authentication | The peer did not provide the authentication key.
//! `TorrentNotWhitelisted` | Authorization | The action cannot be perform on a not-whitelisted torrent (it only applies for trackers running in `listed` or `private_listed` modes).
//!
//! Errors returned by the core tracker.
use std::panic::Location;

use bittorrent_http_protocol::v1::responses;
use bittorrent_primitives::info_hash::InfoHash;
use torrust_tracker_located_error::LocatedError;

use super::authentication::key::ParseKeyError;
use super::databases;

/// Authentication or authorization error returned by the core `Tracker`
/// Whitelist errors returned by the core tracker.
#[derive(thiserror::Error, Debug, Clone)]
pub enum Error {
// Authentication errors
#[error("The supplied key: {key:?}, is not valid: {source}")]
PeerKeyNotValid {
key: super::authentication::Key,
source: LocatedError<'static, dyn std::error::Error + Send + Sync>,
},

#[error("The peer is not authenticated, {location}")]
PeerNotAuthenticated { location: &'static Location<'static> },

// Authorization errors
pub enum WhitelistError {
#[error("The torrent: {info_hash}, is not whitelisted, {location}")]
TorrentNotWhitelisted {
info_hash: InfoHash,
location: &'static Location<'static>,
},
}

/// Errors related to peers keys.
/// Peers keys errors returned by the core tracker.
#[allow(clippy::module_name_repetitions)]
#[derive(thiserror::Error, Debug, Clone)]
pub enum PeerKeyError {
Expand All @@ -55,10 +36,85 @@ pub enum PeerKeyError {
},
}

impl From<Error> for responses::error::Error {
fn from(err: Error) -> Self {
responses::error::Error {
failure_reason: format!("Tracker error: {err}"),
#[cfg(test)]
mod tests {

mod whitelist_error {

use crate::core_tests::sample_info_hash;
use crate::error::WhitelistError;

#[test]
fn torrent_not_whitelisted() {
let err = WhitelistError::TorrentNotWhitelisted {
info_hash: sample_info_hash(),
location: std::panic::Location::caller(),
};

let err_msg = format!("{err}");

assert!(
err_msg.contains(&format!("The torrent: {}, is not whitelisted", sample_info_hash())),
"Error message did not contain expected text: {err_msg}"
);
}
}

mod peer_key_error {
use torrust_tracker_located_error::Located;

use crate::databases::driver::Driver;
use crate::error::PeerKeyError;
use crate::{authentication, databases};

#[test]
fn duration_overflow() {
let seconds_valid = 100;

let err = PeerKeyError::DurationOverflow { seconds_valid };

let err_msg = format!("{err}");

assert!(
err_msg.contains(&format!("Invalid peer key duration: {seconds_valid}")),
"Error message did not contain expected text: {err_msg}"
);
}

#[test]
fn parsing_from_string() {
let err = authentication::key::ParseKeyError::InvalidKeyLength;

let err = PeerKeyError::InvalidKey {
key: "INVALID KEY".to_string(),
source: Located(err).into(),
};

let err_msg = format!("{err}");

assert!(
err_msg.contains(&"Invalid key: INVALID KEY".to_string()),
"Error message did not contain expected text: {err_msg}"
);
}

#[test]
fn persisting_into_database() {
let err = databases::error::Error::InsertFailed {
location: std::panic::Location::caller(),
driver: Driver::Sqlite3,
};

let err = PeerKeyError::DatabaseError {
source: Located(err).into(),
};

let err_msg = format!("{err}");

assert!(
err_msg.contains(&"Can't persist key".to_string()),
"Error message did not contain expected text: {err}"
);
}
}
}
10 changes: 5 additions & 5 deletions packages/tracker-core/src/whitelist/authorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use torrust_tracker_configuration::Core;
use tracing::instrument;

use super::repository::in_memory::InMemoryWhitelist;
use crate::error::Error;
use crate::error::WhitelistError;

pub struct WhitelistAuthorization {
/// Core tracker configuration.
Expand All @@ -32,7 +32,7 @@ impl WhitelistAuthorization {
/// Will return an error if the tracker is running in `listed` mode
/// and the infohash is not whitelisted.
#[instrument(skip(self, info_hash), err)]
pub async fn authorize(&self, info_hash: &InfoHash) -> Result<(), Error> {
pub async fn authorize(&self, info_hash: &InfoHash) -> Result<(), WhitelistError> {
if !self.is_listed() {
return Ok(());
}
Expand All @@ -41,7 +41,7 @@ impl WhitelistAuthorization {
return Ok(());
}

Err(Error::TorrentNotWhitelisted {
Err(WhitelistError::TorrentNotWhitelisted {
info_hash: *info_hash,
location: Location::caller(),
})
Expand Down Expand Up @@ -89,7 +89,7 @@ mod tests {
use torrust_tracker_configuration::Core;

use crate::core_tests::sample_info_hash;
use crate::error::Error;
use crate::error::WhitelistError;
use crate::whitelist::authorization::tests::the_whitelist_authorization_for_announce_and_scrape_actions::{
initialize_whitelist_authorization_and_dependencies_with, initialize_whitelist_authorization_with,
};
Expand Down Expand Up @@ -121,7 +121,7 @@ mod tests {

let result = whitelist_authorization.authorize(&sample_info_hash()).await;

assert!(matches!(result.unwrap_err(), Error::TorrentNotWhitelisted { .. }));
assert!(matches!(result.unwrap_err(), WhitelistError::TorrentNotWhitelisted { .. }));
}
}

Expand Down

0 comments on commit c1586de

Please sign in to comment.