Merge #1252: Overhaul core Tracker: add tests for error mod

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

Author: josecelano

Cargo.lock

@@ -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"

packages/http-protocol/Cargo.toml

@@ -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 {
 

packages/http-protocol/src/v1/responses/error.rs

@@ -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"] }

packages/tracker-core/Cargo.toml

@@ -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 {
@@ -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}"
+            );
         }
     }
 }

packages/tracker-core/src/error.rs

@@ -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.
@@ -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(());
         }
@@ -41,7 +41,7 @@ impl WhitelistAuthorization {
             return Ok(());
         }
 
-        Err(Error::TorrentNotWhitelisted {
+        Err(WhitelistError::TorrentNotWhitelisted {
             info_hash: *info_hash,
             location: Location::caller(),
         })
@@ -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,
             };
@@ -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 { .. }));
             }
         }