fix: [#1264] partially. The correct number of downloads is persited

However, we still have to load the value counter from the database the
first time the otrrent is added to the repository.

Author: josecelano

packages/tracker-core/src/announce_handler.rs

@@ -97,7 +97,6 @@ use bittorrent_primitives::info_hash::InfoHash;
 use torrust_tracker_configuration::{Core, TORRENT_PEERS_LIMIT};
 use torrust_tracker_primitives::core::AnnounceData;
 use torrust_tracker_primitives::peer;
-use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
 
 use super::torrent::repository::in_memory::InMemoryTorrentRepository;
 use super::torrent::repository::persisted::DatabasePersistentTorrentRepository;
@@ -164,45 +163,29 @@ impl AnnounceHandler {
     ) -> Result<AnnounceData, AnnounceError> {
         self.whitelist_authorization.authorize(info_hash).await?;
 
-        tracing::debug!("Before: {peer:?}");
         peer.change_ip(&assign_ip_address_to_peer(remote_client_ip, self.config.net.external_ip));
-        tracing::debug!("After: {peer:?}");
 
-        let stats = self.upsert_peer_and_get_stats(info_hash, peer);
-
-        let peers = self
-            .in_memory_torrent_repository
-            .get_peers_for(info_hash, peer, peers_wanted.limit());
-
-        Ok(AnnounceData {
-            peers,
-            stats,
-            policy: self.config.announce_policy,
-        })
-    }
-
-    /// Updates the torrent data in memory, persists statistics if needed, and
-    /// returns the updated swarm stats.
-    #[must_use]
-    fn upsert_peer_and_get_stats(&self, info_hash: &InfoHash, peer: &peer::Peer) -> SwarmMetadata {
         let number_of_downloads_increased = self.in_memory_torrent_repository.upsert_peer(info_hash, peer);
 
-        let swarm_metadata = self.in_memory_torrent_repository.get_swarm_metadata(info_hash);
-
-        if number_of_downloads_increased {
-            self.persist_stats(info_hash, &swarm_metadata);
+        if self.config.tracker_policy.persistent_torrent_completed_stat && number_of_downloads_increased {
+            self.db_torrent_repository.increase_number_of_downloads(info_hash)?;
         }
 
-        swarm_metadata
+        Ok(self.build_announce_data(info_hash, peer, peers_wanted))
     }
 
-    /// Persists torrent statistics to the database if persistence is enabled.
-    fn persist_stats(&self, info_hash: &InfoHash, swarm_metadata: &SwarmMetadata) {
-        if self.config.tracker_policy.persistent_torrent_completed_stat {
-            let completed = swarm_metadata.downloaded;
-            let info_hash = *info_hash;
+    /// Builds the announce data for the peer making the request.
+    fn build_announce_data(&self, info_hash: &InfoHash, peer: &peer::Peer, peers_wanted: &PeersWanted) -> AnnounceData {
+        let peers = self
+            .in_memory_torrent_repository
+            .get_peers_for(info_hash, peer, peers_wanted.limit());
 
-            drop(self.db_torrent_repository.save(&info_hash, completed));
+        let swarm_metadata = self.in_memory_torrent_repository.get_swarm_metadata(info_hash);
+
+        AnnounceData {
+            peers,
+            stats: swarm_metadata,
+            policy: self.config.announce_policy,
         }
     }
 }

packages/tracker-core/src/databases/driver/sqlite.rs

@@ -164,19 +164,12 @@ impl Database for Sqlite {
     fn increase_number_of_downloads(&self, info_hash: &InfoHash) -> Result<(), Error> {
         let conn = self.pool.get().map_err(|e| (e, DRIVER))?;
 
-        let update = conn.execute(
+        let _ = conn.execute(
             "UPDATE torrents SET completed = completed + 1 WHERE info_hash = ?",
             [info_hash.to_string()],
         )?;
 
-        if update == 0 {
-            Err(Error::UpdateFailed {
-                location: Location::caller(),
-                driver: DRIVER,
-            })
-        } else {
-            Ok(())
-        }
+        Ok(())
     }
 
     /// Refer to [`databases::Database::load_keys`](crate::core::databases::Database::load_keys).

packages/tracker-core/src/databases/mod.rs

@@ -128,6 +128,9 @@ pub trait Database: Sync + Send {
 
     /// Increases the number of downloads for a given torrent.
     ///
+    /// It does not create a new entry if the torrent is not found and it does
+    /// not return an error.
+    ///
     /// # Arguments
     ///
     /// * `info_hash` - A reference to the torrent's info hash.

packages/tracker-core/src/error.rs

@@ -66,6 +66,10 @@ pub enum AnnounceError {
     /// Wraps errors related to torrent whitelisting.
     #[error("Whitelist error: {0}")]
     Whitelist(#[from] WhitelistError),
+
+    /// Wraps errors related to database.
+    #[error("Database error: {0}")]
+    Database(#[from] databases::error::Error),
 }
 
 /// Errors related to scrape requests.

packages/tracker-core/src/torrent/repository/persisted.rs

@@ -47,6 +47,26 @@ impl DatabasePersistentTorrentRepository {
         }
     }
 
+    /// Increases the number of downloads for a given torrent.
+    ///
+    /// If the torrent is not found, it creates a new entry.
+    ///
+    /// # Arguments
+    ///
+    /// * `info_hash` - The info hash of the torrent.
+    ///
+    /// # Errors
+    ///
+    /// Returns an [`Error`] if the database operation fails.
+    pub(crate) fn increase_number_of_downloads(&self, info_hash: &InfoHash) -> Result<(), Error> {
+        let torrent = self.load(info_hash)?;
+
+        match torrent {
+            Some(_number_of_downloads) => self.database.increase_number_of_downloads(info_hash),
+            None => self.save(info_hash, 1),
+        }
+    }
+
     /// Loads all persistent torrent metrics from the database.
     ///
     /// This function retrieves the torrent metrics (e.g., download counts) from the persistent store
@@ -67,7 +87,6 @@ impl DatabasePersistentTorrentRepository {
     /// # Errors
     ///
     /// Returns an [`Error`] if the underlying database query fails.
-    #[allow(dead_code)]
     pub(crate) fn load(&self, info_hash: &InfoHash) -> Result<Option<PersistentTorrent>, Error> {
         self.database.load_persistent_torrent(info_hash)
     }
@@ -118,6 +137,19 @@ mod tests {
         assert_eq!(torrents.get(&infohash), Some(1).as_ref());
     }
 
+    #[test]
+    fn it_increases_the_numbers_of_downloads_for_a_torrent_into_the_database() {
+        let repository = initialize_db_persistent_torrent_repository();
+
+        let infohash = sample_info_hash();
+
+        repository.increase_number_of_downloads(&infohash).unwrap();
+
+        let torrents = repository.load_all().unwrap();
+
+        assert_eq!(torrents.get(&infohash), Some(1).as_ref());
+    }
+
     #[test]
     fn it_loads_the_numbers_of_downloads_for_all_torrents_from_the_database() {
         let repository = initialize_db_persistent_torrent_repository();