Merge #305: Refactor: the function `services::torrent::Index::add_tor…

…rent`

275231a doc: [#276] add comment for upload torrent secondary tasks (Jose Celano)
f19e801 refactor: [#276] extract fn Torrent::import_torrent_statistics_from_tracker (Jose Celano)
fbb42ad feat!: [#276] do not persist uploaded torrent if it cannot persit tags (Jose Celano)
6a75c54 refactor: [#276] extract fn Torrent::canonical_info_hash_group_checks (Jose Celano)
bbadd59 refactor: [#276] extract fn Torrent::customize_announcement_info_for (Jose Celano)
4cc97c7 feat!: [#276] upload torrent returns 400 instead of empty response (Jose Celano)
329485f refactor: [#276] extract fn parse_torrent::decode_and_validate_torrent_file (Jose Celano)
a46d300 refactor: [#276] extract function Torrent::validate_and_build_metadata (Jose Celano)
ca6e97c refactor: [#276] move metadata format validation to Metadata struct (Jose Celano)
a302c22 test: [#276] add more tests for torrent upload (Jose Celano)
7d50a17 refactor: reorganize test mods for torrent contract (Jose Celano)

Pull request description:

  - [x] Reorganize tests in torrent context contract.
  - [x] Add more tests before the refactoring.
  - [x] Refactorings.

ACKs for top commit:
  josecelano:
    ACK 275231a

Tree-SHA512: 46397926dbd9def23e2ad233fe0eec40f5c16a008a8ef14ad3803776b351faf291c89560536a9d67d20490a9030a3f6c9a5a01b6ef1e9a750388c6dbc79a4847

src/databases/database.rs

@@ -7,7 +7,7 @@ use crate::databases::sqlite::Sqlite;
 use crate::models::category::CategoryId;
 use crate::models::info_hash::InfoHash;
 use crate::models::response::TorrentsResponse;
-use crate::models::torrent::TorrentListing;
+use crate::models::torrent::{Metadata, TorrentListing};
 use crate::models::torrent_file::{DbTorrent, Torrent, TorrentFile};
 use crate::models::torrent_tag::{TagId, TorrentTag};
 use crate::models::tracker_key::TrackerKey;
@@ -196,9 +196,7 @@ pub trait Database: Sync + Send {
         original_info_hash: &InfoHash,
         torrent: &Torrent,
         uploader_id: UserId,
-        category_id: i64,
-        title: &str,
-        description: &str,
+        metadata: &Metadata,
     ) -> Result<i64, Error>;
 
     /// Get `Torrent` from `InfoHash`.

src/databases/mysql.rs

@@ -12,7 +12,7 @@ use crate::databases::database::{Category, Database, Driver, Sorting, TorrentCom
 use crate::models::category::CategoryId;
 use crate::models::info_hash::InfoHash;
 use crate::models::response::TorrentsResponse;
-use crate::models::torrent::TorrentListing;
+use crate::models::torrent::{Metadata, TorrentListing};
 use crate::models::torrent_file::{DbTorrent, DbTorrentAnnounceUrl, DbTorrentFile, Torrent, TorrentFile};
 use crate::models::torrent_tag::{TagId, TorrentTag};
 use crate::models::tracker_key::TrackerKey;
@@ -432,11 +432,9 @@ impl Database for Mysql {
         original_info_hash: &InfoHash,
         torrent: &Torrent,
         uploader_id: UserId,
-        category_id: i64,
-        title: &str,
-        description: &str,
+        metadata: &Metadata,
     ) -> Result<i64, database::Error> {
-        let info_hash = torrent.info_hash_hex();
+        let info_hash = torrent.canonical_info_hash_hex();
         let canonical_info_hash = torrent.canonical_info_hash();
 
         // open pool connection
@@ -471,7 +469,7 @@ impl Database for Mysql {
         ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, UTC_TIMESTAMP())",
         )
         .bind(uploader_id)
-        .bind(category_id)
+        .bind(metadata.category_id)
         .bind(info_hash.to_lowercase())
         .bind(torrent.file_size())
         .bind(torrent.info.name.to_string())
@@ -585,11 +583,28 @@ impl Database for Mysql {
             return Err(e);
         }
 
+        // Insert tags
+
+        for tag_id in &metadata.tags {
+            let insert_torrent_tag_result = query("INSERT INTO torrust_torrent_tag_links (torrent_id, tag_id) VALUES (?, ?)")
+                .bind(torrent_id)
+                .bind(tag_id)
+                .execute(&mut tx)
+                .await
+                .map_err(|err| database::Error::ErrorWithText(err.to_string()));
+
+            // rollback transaction on error
+            if let Err(e) = insert_torrent_tag_result {
+                drop(tx.rollback().await);
+                return Err(e);
+            }
+        }
+
         let insert_torrent_info_result =
             query(r#"INSERT INTO torrust_torrent_info (torrent_id, title, description) VALUES (?, ?, NULLIF(?, ""))"#)
                 .bind(torrent_id)
-                .bind(title)
-                .bind(description)
+                .bind(metadata.title.clone())
+                .bind(metadata.description.clone())
                 .execute(&mut tx)
                 .await
                 .map_err(|e| match e {

src/databases/sqlite.rs

@@ -12,7 +12,7 @@ use crate::databases::database::{Category, Database, Driver, Sorting, TorrentCom
 use crate::models::category::CategoryId;
 use crate::models::info_hash::InfoHash;
 use crate::models::response::TorrentsResponse;
-use crate::models::torrent::TorrentListing;
+use crate::models::torrent::{Metadata, TorrentListing};
 use crate::models::torrent_file::{DbTorrent, DbTorrentAnnounceUrl, DbTorrentFile, Torrent, TorrentFile};
 use crate::models::torrent_tag::{TagId, TorrentTag};
 use crate::models::tracker_key::TrackerKey;
@@ -422,11 +422,9 @@ impl Database for Sqlite {
         original_info_hash: &InfoHash,
         torrent: &Torrent,
         uploader_id: UserId,
-        category_id: i64,
-        title: &str,
-        description: &str,
+        metadata: &Metadata,
     ) -> Result<i64, database::Error> {
-        let info_hash = torrent.info_hash_hex();
+        let info_hash = torrent.canonical_info_hash_hex();
         let canonical_info_hash = torrent.canonical_info_hash();
 
         // open pool connection
@@ -461,7 +459,7 @@ impl Database for Sqlite {
         ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, strftime('%Y-%m-%d %H:%M:%S',DATETIME('now', 'utc')))",
         )
         .bind(uploader_id)
-        .bind(category_id)
+        .bind(metadata.category_id)
         .bind(info_hash.to_lowercase())
         .bind(torrent.file_size())
         .bind(torrent.info.name.to_string())
@@ -575,11 +573,28 @@ impl Database for Sqlite {
             return Err(e);
         }
 
+        // Insert tags
+
+        for tag_id in &metadata.tags {
+            let insert_torrent_tag_result = query("INSERT INTO torrust_torrent_tag_links (torrent_id, tag_id) VALUES (?, ?)")
+                .bind(torrent_id)
+                .bind(tag_id)
+                .execute(&mut tx)
+                .await
+                .map_err(|err| database::Error::ErrorWithText(err.to_string()));
+
+            // rollback transaction on error
+            if let Err(e) = insert_torrent_tag_result {
+                drop(tx.rollback().await);
+                return Err(e);
+            }
+        }
+
         let insert_torrent_info_result =
             query(r#"INSERT INTO torrust_torrent_info (torrent_id, title, description) VALUES (?, ?, NULLIF(?, ""))"#)
                 .bind(torrent_id)
-                .bind(title)
-                .bind(description)
+                .bind(metadata.title.clone())
+                .bind(metadata.description.clone())
                 .execute(&mut tx)
                 .await
                 .map_err(|e| match e {

src/errors.rs

@@ -5,6 +5,8 @@ use derive_more::{Display, Error};
 use hyper::StatusCode;
 
 use crate::databases::database;
+use crate::models::torrent::MetadataError;
+use crate::utils::parse_torrent::DecodeTorrentFileError;
 
 pub type ServiceResult<V> = Result<V, ServiceError>;
 
@@ -204,6 +206,28 @@ impl From<serde_json::Error> for ServiceError {
     }
 }
 
+impl From<MetadataError> for ServiceError {
+    fn from(e: MetadataError) -> Self {
+        eprintln!("{e}");
+        match e {
+            MetadataError::MissingTorrentTitle => ServiceError::MissingMandatoryMetadataFields,
+            MetadataError::InvalidTorrentTitleLength => ServiceError::InvalidTorrentTitleLength,
+        }
+    }
+}
+
+impl From<DecodeTorrentFileError> for ServiceError {
+    fn from(e: DecodeTorrentFileError) -> Self {
+        eprintln!("{e}");
+        match e {
+            DecodeTorrentFileError::InvalidTorrentPiecesLength => ServiceError::InvalidTorrentTitleLength,
+            DecodeTorrentFileError::CannotBencodeInfoDict
+            | DecodeTorrentFileError::InvalidInfoDictionary
+            | DecodeTorrentFileError::InvalidBencodeData => ServiceError::InvalidTorrentFile,
+        }
+    }
+}
+
 #[must_use]
 pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
     #[allow(clippy::match_same_arms)]

src/models/torrent.rs

@@ -1,7 +1,10 @@
+use derive_more::{Display, Error};
 use serde::{Deserialize, Serialize};
 
+use super::category::CategoryId;
 use super::torrent_tag::TagId;
-use crate::errors::ServiceError;
+
+const MIN_TORRENT_TITLE_LENGTH: usize = 3;
 
 #[allow(clippy::module_name_repetitions)]
 pub type TorrentId = i64;
@@ -24,25 +27,67 @@ pub struct TorrentListing {
     pub comment: Option<String>,
 }
 
+#[derive(Debug, Display, PartialEq, Eq, Error)]
+pub enum MetadataError {
+    #[display(fmt = "Missing mandatory torrent title.")]
+    MissingTorrentTitle,
+
+    #[display(fmt = "Torrent title is too short.")]
+    InvalidTorrentTitleLength,
+}
+
 #[derive(Debug, Deserialize)]
 pub struct Metadata {
     pub title: String,
     pub description: String,
-    pub category: String,
+    pub category_id: CategoryId,
     pub tags: Vec<TagId>,
 }
 
 impl Metadata {
-    /// Returns the verify of this [`Metadata`].
+    /// Create a new struct.
     ///
     /// # Errors
     ///
-    /// This function will return an error if the any of the mandatory metadata fields are missing.
-    pub fn verify(&self) -> Result<(), ServiceError> {
-        if self.title.is_empty() || self.category.is_empty() {
-            Err(ServiceError::MissingMandatoryMetadataFields)
-        } else {
-            Ok(())
+    /// This function will return an error if the metadata fields do not have a
+    /// valid format.
+    pub fn new(title: &str, description: &str, category_id: CategoryId, tag_ids: &[TagId]) -> Result<Self, MetadataError> {
+        Self::validate_format(title, description, category_id, tag_ids)?;
+
+        Ok(Self {
+            title: title.to_owned(),
+            description: description.to_owned(),
+            category_id,
+            tags: tag_ids.to_vec(),
+        })
+    }
+
+    /// It validates the format of the metadata fields.
+    ///
+    /// It does not validate domain rules, like:
+    ///
+    /// - Duplicate titles.
+    /// - Non-existing categories.
+    /// - ...
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if any of the metadata fields does
+    /// not have a valid format.
+    fn validate_format(
+        title: &str,
+        _description: &str,
+        _category_id: CategoryId,
+        _tag_ids: &[TagId],
+    ) -> Result<(), MetadataError> {
+        if title.is_empty() {
+            return Err(MetadataError::MissingTorrentTitle);
         }
+
+        if title.len() < MIN_TORRENT_TITLE_LENGTH {
+            return Err(MetadataError::InvalidTorrentTitleLength);
+        }
+
+        Ok(())
     }
 }

src/models/torrent_file.rs

@@ -4,7 +4,6 @@ use serde_bytes::ByteBuf;
 use sha1::{Digest, Sha1};
 
 use super::info_hash::InfoHash;
-use crate::config::Configuration;
 use crate::utils::hex::{from_bytes, into_bytes};
 
 #[derive(PartialEq, Debug, Clone, Serialize, Deserialize)]
@@ -101,19 +100,25 @@ impl Torrent {
         }
     }
 
-    /// Sets the announce url to the tracker url and removes all other trackers
-    /// if the torrent is private.
-    pub async fn set_announce_urls(&mut self, cfg: &Configuration) {
-        let settings = cfg.settings.read().await;
+    /// Sets the announce url to the tracker url.
+    pub fn set_announce_to(&mut self, tracker_url: &str) {
+        self.announce = Some(tracker_url.to_owned());
+    }
 
-        self.announce = Some(settings.tracker.url.clone());
+    /// Removes all other trackers if the torrent is private.
+    pub fn reset_announce_list_if_private(&mut self) {
+        if self.is_private() {
+            self.announce_list = None;
+        }
+    }
 
-        // if torrent is private, remove all other trackers
+    fn is_private(&self) -> bool {
         if let Some(private) = self.info.private {
             if private == 1 {
-                self.announce_list = None;
+                return true;
             }
         }
+        false
     }
 
     /// It calculates the info hash of the torrent file.
@@ -133,13 +138,13 @@ impl Torrent {
     }
 
     #[must_use]
-    pub fn info_hash_hex(&self) -> String {
-        from_bytes(&self.calculate_info_hash_as_bytes()).to_lowercase()
+    pub fn canonical_info_hash(&self) -> InfoHash {
+        self.calculate_info_hash_as_bytes().into()
     }
 
     #[must_use]
-    pub fn canonical_info_hash(&self) -> InfoHash {
-        self.calculate_info_hash_as_bytes().into()
+    pub fn canonical_info_hash_hex(&self) -> String {
+        self.canonical_info_hash().to_hex_string()
     }
 
     #[must_use]
@@ -389,7 +394,7 @@ mod tests {
                 httpseeds: None,
             };
 
-            assert_eq!(torrent.info_hash_hex(), "79fa9e4a2927804fe4feab488a76c8c2d3d1cdca");
+            assert_eq!(torrent.canonical_info_hash_hex(), "79fa9e4a2927804fe4feab488a76c8c2d3d1cdca");
         }
 
         mod infohash_should_be_calculated_for {
@@ -430,7 +435,7 @@ mod tests {
                     httpseeds: None,
                 };
 
-                assert_eq!(torrent.info_hash_hex(), "79fa9e4a2927804fe4feab488a76c8c2d3d1cdca");
+                assert_eq!(torrent.canonical_info_hash_hex(), "79fa9e4a2927804fe4feab488a76c8c2d3d1cdca");
             }
 
             #[test]
@@ -469,7 +474,7 @@ mod tests {
                     httpseeds: None,
                 };
 
-                assert_eq!(torrent.info_hash_hex(), "aa2aca91ab650c4d249c475ca3fa604f2ccb0d2a");
+                assert_eq!(torrent.canonical_info_hash_hex(), "aa2aca91ab650c4d249c475ca3fa604f2ccb0d2a");
             }
 
             #[test]
@@ -504,7 +509,7 @@ mod tests {
                     httpseeds: None,
                 };
 
-                assert_eq!(torrent.info_hash_hex(), "ccc1cf4feb59f3fa85c96c9be1ebbafcfe8a9cc8");
+                assert_eq!(torrent.canonical_info_hash_hex(), "ccc1cf4feb59f3fa85c96c9be1ebbafcfe8a9cc8");
             }
 
             #[test]
@@ -539,7 +544,7 @@ mod tests {
                     httpseeds: None,
                 };
 
-                assert_eq!(torrent.info_hash_hex(), "d3a558d0a19aaa23ba6f9f430f40924d10fefa86");
+                assert_eq!(torrent.canonical_info_hash_hex(), "d3a558d0a19aaa23ba6f9f430f40924d10fefa86");
             }
         }
     }