Refactor packages: extract announce and scrape service/handler structs

[josecelano]

There are some functions including this attribute #[allow(clippy::too_many_arguments)]. They are announce and scrape handlers at different app layers.

HTTP

axum-http-tracker-serverpackage:

• torrust_axum_http_tracker_server::v1::handlers::announce::handle
• torrust_axum_http_tracker_server::v1::handlers::announce::handle_announce
• torrust_axum_http_tracker_server::v1::handlers::scrape::handle
• torrust_axum_http_tracker_server::v1::handlers::scrape::handle_scrape

http-tracker-core package:

• bittorrent_http_tracker_core::services::announce::handle_announce
• bittorrent_http_tracker_core::services::scrape::handle_scrape

UDP

udp-tracker-core package:

• bittorrent_udp_tracker_core::services::announce::handle_announce
• bittorrent_udp_tracker_core::services::scrape::handle_scrape (this does not have the too_many_arguments attribute)

udp-tracker-server package:

• torrust_udp_tracker_server::handlers::announce::handle_announce
• torrust_udp_tracker_server::handlers::scrape::handle:scrape (this does not have the too_many_arguments attribute)
• torrust_udp_tracker_server::handlers::error::handle_error

They have many arguments because the services/handlers are implemented as functions not types. So we need to pass all the dependencies to the function from the main AppContainer down to the service/handler function. We can create structs and pass the dependencies in the constructor. That will simplify the announce/scrape functions, improving the core and removing the clippy warning.

AI generated example

As an example I asked an AI model to refactor one of the modules (bittorrent_http_tracker_core::services::announce).

NOTE: I have not only asked to create the struct but also to improve the code in the main function. We can do both things as there is no extra effort and It will be good when we start adding unit tests to all the packages.

AI model: deepseek/deepseek-r1

Prompt

Can you refactor the following Rust module. I want to:

- Reduce the number of parameters in the handle_announce function
- Convert the function into a struct with a constructor
- Extract functionality of that function into other internal private methods

THE MODULE

Response

//! The `announce` service.
//!
//! The service is responsible for handling the `announce` requests.
//!
//! It delegates the `announce` logic to the [`AnnounceHandler`] and it returns
//! the [`AnnounceData`].
//!
//! It also sends an [`http_tracker_core::statistics::event::Event`]
//! because events are specific for the HTTP tracker.
use std::net::IpAddr;
use std::panic::Location;
use std::sync::Arc;

use bittorrent_http_tracker_protocol::v1::requests::announce::{peer_from_request, Announce};
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::{self, ClientIpSources, PeerIpResolutionError};
use bittorrent_tracker_core::announce_handler::{AnnounceHandler, PeersWanted};
use bittorrent_tracker_core::authentication::service::AuthenticationService;
use bittorrent_tracker_core::authentication::{self, Key};
use bittorrent_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistError};
use bittorrent_tracker_core::whitelist;
use torrust_tracker_configuration::Core;
use torrust_tracker_primitives::core::AnnounceData;

use crate::statistics;

/// Errors related to announce requests.
#[derive(thiserror::Error, Debug, Clone)]
pub enum HttpAnnounceError {
    #[error("Error resolving peer IP: {source}")]
    PeerIpResolutionError { source: PeerIpResolutionError },

    #[error("Tracker core error: {source}")]
    TrackerCoreError { source: TrackerCoreError },
}

impl From<PeerIpResolutionError> for HttpAnnounceError {
    fn from(peer_ip_resolution_error: PeerIpResolutionError) -> Self {
        Self::PeerIpResolutionError {
            source: peer_ip_resolution_error,
        }
    }
}

impl From<TrackerCoreError> for HttpAnnounceError {
    fn from(tracker_core_error: TrackerCoreError) -> Self {
        Self::TrackerCoreError {
            source: tracker_core_error,
        }
    }
}

impl From<AnnounceError> for HttpAnnounceError {
    fn from(announce_error: AnnounceError) -> Self {
        Self::TrackerCoreError {
            source: announce_error.into(),
        }
    }
}

impl From<WhitelistError> for HttpAnnounceError {
    fn from(whitelist_error: WhitelistError) -> Self {
        Self::TrackerCoreError {
            source: whitelist_error.into(),
        }
    }
}

impl From<authentication::key::Error> for HttpAnnounceError {
    fn from(whitelist_error: authentication::key::Error) -> Self {
        Self::TrackerCoreError {
            source: whitelist_error.into(),
        }
    }
}

/// The HTTP tracker `announce` service.
///
/// The service sends an statistics event that increments:
///
/// - The number of TCP connections handled by the HTTP tracker.
/// - The number of TCP `announce` requests handled by the HTTP tracker.
///
/// > **NOTICE**: as the HTTP tracker does not requires a connection request
/// > like the UDP tracker, the number of TCP connections is incremented for
/// > each `announce` request.
pub struct AnnounceService {
    core_config: Arc<Core>,
    announce_handler: Arc<AnnounceHandler>,
    authentication_service: Arc<AuthenticationService>,
    whitelist_authorization: Arc<whitelist::authorization::WhitelistAuthorization>,
    opt_http_stats_event_sender: Arc<Option<Box<dyn statistics::event::sender::Sender>>>,
}

impl AnnounceService {
    /// Creates a new [`AnnounceService`] with the given dependencies
    pub fn new(
        core_config: Arc<Core>,
        announce_handler: Arc<AnnounceHandler>,
        authentication_service: Arc<AuthenticationService>,
        whitelist_authorization: Arc<whitelist::authorization::WhitelistAuthorization>,
        opt_http_stats_event_sender: Arc<Option<Box<dyn statistics::event::sender::Sender>>>,
    ) -> Self {
        Self {
            core_config,
            announce_handler,
            authentication_service,
            whitelist_authorization,
            opt_http_stats_event_sender,
        }
    }

    /// Handles an announce request
    ///
    /// # Errors
    ///
    /// This function will return an error if:
    ///
    /// - The tracker is running in `listed` mode and the torrent is not whitelisted.
    /// - There is an error when resolving the client IP address.
    pub async fn handle_announce(
        &self,
        announce_request: &Announce,
        client_ip_sources: &ClientIpSources,
        maybe_key: Option<Key>,
    ) -> Result<AnnounceData, HttpAnnounceError> {
        self.authenticate(maybe_key).await?;
        self.authorize(&announce_request.info_hash).await?;

        let peer_ip = self.resolve_peer_ip(client_ip_sources)?;
        let mut peer = Self::build_peer(announce_request, &peer_ip);
        let peers_wanted = self.get_peers_wanted(announce_request.numwant);
        let original_peer_ip = peer.peer_addr.ip();

        let announce_data = self
            .announce(&announce_request.info_hash, &mut peer, &original_peer_ip, &peers_wanted)
            .await?;

        self.send_statistics_event(original_peer_ip).await;

        Ok(announce_data)
    }

    async fn authenticate(&self, maybe_key: Option<Key>) -> Result<(), HttpAnnounceError> {
        if self.core_config.private {
            let key = maybe_key.ok_or(authentication::key::Error::MissingAuthKey {
                location: Location::caller(),
            })?;

            self.authentication_service.authenticate(&key).await?;
        }
        Ok(())
    }

    async fn authorize(&self, info_hash: &[u8; 20]) -> Result<(), HttpAnnounceError> {
        self.whitelist_authorization.authorize(info_hash).await?;
        Ok(())
    }

    fn resolve_peer_ip(&self, client_ip_sources: &ClientIpSources) -> Result<IpAddr, HttpAnnounceError> {
        Ok(peer_ip_resolver::invoke(
            self.core_config.net.on_reverse_proxy,
            client_ip_sources,
        )?)
    }

    fn build_peer(announce_request: &Announce, peer_ip: &IpAddr) -> Peer {
        peer_from_request(announce_request, peer_ip)
    }

    fn get_peers_wanted(&self, numwant: Option<u32>) -> PeersWanted {
        numwant.map_or(PeersWanted::AsManyAsPossible, PeersWanted::only)
    }

    async fn announce(
        &self,
        info_hash: &[u8; 20],
        peer: &mut Peer,
        original_peer_ip: &IpAddr,
        peers_wanted: &PeersWanted,
    ) -> Result<AnnounceData, HttpAnnounceError> {
        Ok(self
            .announce_handler
            .announce(info_hash, peer, original_peer_ip, peers_wanted)
            .await?)
    }

    async fn send_statistics_event(&self, original_peer_ip: IpAddr) {
        if let Some(http_stats_event_sender) = self.opt_http_stats_event_sender.as_deref() {
            match original_peer_ip {
                IpAddr::V4(_) => {
                    http_stats_event_sender
                        .send_event(statistics::event::Event::Tcp4Announce)
                        .await;
                }
                IpAddr::V6(_) => {
                    http_stats_event_sender
                        .send_event(statistics::event::Event::Tcp6Announce)
                        .await;
                }
            }
        }
    }
}

#[cfg(test)]
mod tests {
    // ... (tests remain mostly the same except for changing handle_announce calls to use AnnounceService)
    use super::*;
    
    #[tokio::test]
    async fn it_should_return_the_announce_data() {
        let (core_tracker_services, core_http_tracker_services) = initialize_core_tracker_services();

        let peer = sample_peer();
        let (announce_request, client_ip_sources) = sample_announce_request_for_peer(peer);

        let announce_service = AnnounceService::new(
            core_tracker_services.core_config,
            core_tracker_services.announce_handler,
            core_tracker_services.authentication_service,
            core_tracker_services.whitelist_authorization,
            core_http_tracker_services.http_stats_event_sender,
        );

        let announce_data = announce_service
            .handle_announce(&announce_request, &client_ip_sources, None)
            .await
            .unwrap();

        // ... rest of test assertion
    }
    
    // Other tests updated similarly...
}

Key changes made:

- Created AnnounceService struct to encapsulate dependencies
- Moved core functionality into private methods (authenticate, authorize, etc.)
- Reduced handle_announce parameters from 8 to 3
- Maintained original functionality while improving readability
- Updated tests to use the new struct-based API
- Improved error handling flow using Rust's ? operator
- Split logic into distinct, testable components while maintaining the original error conversions

The tests would need to be updated to create an instance of AnnounceService using the same dependencies that were previously passed individually to the free function. All test assertions should remain the same since the functionality is unchanged.

cc @da2ce7