Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#670] new JSON serialization for connect and error aquatic responses #876

Conversation

mario-nt
Copy link
Contributor

Resolves #670

Copy link

codecov bot commented May 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 55 lines in your changes missing coverage. Please review.

Project coverage is 78.54%. Comparing base (a0a7056) to head (0157d96).

Files Patch % Lines
src/console/clients/udp/responses/dto.rs 0.00% 51 Missing ⚠️
src/console/clients/udp/responses/json.rs 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #876      +/-   ##
===========================================
- Coverage    78.58%   78.54%   -0.05%     
===========================================
  Files          170      171       +1     
  Lines         9429     9434       +5     
===========================================
  Hits          7410     7410              
- Misses        2019     2024       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@josecelano josecelano added the Needs Rebase Base Branch has Incompatibilities label May 21, 2024
@mario-nt mario-nt force-pushed the 670-UDP-tracker-client-print-unexpected-responses-JSON branch from cff3d70 to 3c78be9 Compare May 21, 2024 19:11
@mario-nt mario-nt marked this pull request as ready for review May 21, 2024 19:50
@mario-nt mario-nt requested a review from josecelano May 21, 2024 19:50
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt I think now it's possible to simplify the print_response function because we have implemented all cases and we can map normal responses to DTO responses.

This is only an example (generated by ChatGPT) not tested:

use std::net::{SocketAddr, ToSocketAddrs};
use std::str::FromStr;

use anyhow::Context;
use aquatic_udp_protocol::Response::{self, AnnounceIpv4, AnnounceIpv6, Connect, Error, Scrape};
use aquatic_udp_protocol::{Port, TransactionId};
use clap::{Parser, Subcommand};
use log::{debug, LevelFilter};
use torrust_tracker_primitives::info_hash::InfoHash as TorrustInfoHash;
use url::Url;

use crate::console::clients::udp::checker;
use crate::console::clients::udp::responses::{AnnounceResponseDto, ConnectResponseDto, ErrorResponseDto, ScrapeResponseDto};

const ASSIGNED_BY_OS: u16 = 0;
const RANDOM_TRANSACTION_ID: i32 = -888_840_697;

#[derive(Parser, Debug)]
#[command(author, version, about, long_about = None)]
struct Args {
    #[command(subcommand)]
    command: Command,
}

#[derive(Subcommand, Debug)]
enum Command {
    Announce {
        #[arg(value_parser = parse_socket_addr)]
        tracker_socket_addr: SocketAddr,
        #[arg(value_parser = parse_info_hash)]
        info_hash: TorrustInfoHash,
    },
    Scrape {
        #[arg(value_parser = parse_socket_addr)]
        tracker_socket_addr: SocketAddr,
        #[arg(value_parser = parse_info_hash, num_args = 1..=74, value_delimiter = ' ')]
        info_hashes: Vec<TorrustInfoHash>,
    },
}

/// # Errors
///
/// Will return an error if the command fails.
///
pub async fn run() -> anyhow::Result<()> {
    setup_logging(LevelFilter::Info);

    let args = Args::parse();

    let response = match args.command {
        Command::Announce {
            tracker_socket_addr,
            info_hash,
        } => handle_announce(&tracker_socket_addr, &info_hash).await?,
        Command::Scrape {
            tracker_socket_addr,
            info_hashes,
        } => handle_scrape(&tracker_socket_addr, &info_hashes).await?,
    };

    print_response(response)
}

fn setup_logging(level: LevelFilter) {
    if let Err(_err) = fern::Dispatch::new()
        .format(|out, message, record| {
            out.finish(format_args!(
                "{} [{}][{}] {}",
                chrono::Local::now().format("%+"),
                record.target(),
                record.level(),
                message
            ));
        })
        .level(level)
        .chain(std::io::stdout())
        .apply()
    {
        panic!("Failed to initialize logging.")
    }

    debug!("logging initialized.");
}

async fn handle_announce(tracker_socket_addr: &SocketAddr, info_hash: &TorrustInfoHash) -> anyhow::Result<Response> {
    let transaction_id = TransactionId::new(RANDOM_TRANSACTION_ID);

    let mut client = checker::Client::default();

    let bound_to = client.bind_and_connect(ASSIGNED_BY_OS, tracker_socket_addr).await?;

    let connection_id = client.send_connection_request(transaction_id).await?;

    client
        .send_announce_request(connection_id, transaction_id, *info_hash, Port(bound_to.port().into()))
        .await
}

async fn handle_scrape(tracker_socket_addr: &SocketAddr, info_hashes: &[TorrustInfoHash]) -> anyhow::Result<Response> {
    let transaction_id = TransactionId::new(RANDOM_TRANSACTION_ID);

    let mut client = checker::Client::default();

    let _bound_to = client.bind_and_connect(ASSIGNED_BY_OS, tracker_socket_addr).await?;

    let connection_id = client.send_connection_request(transaction_id).await?;

    client
        .send_scrape_request(connection_id, transaction_id, info_hashes.to_vec())
        .await
}

fn print_response(response: Response) -> anyhow::Result<()> {
    let response_dto: ResponseDto = response.into();
    let pretty_json = serde_json::to_string_pretty(&response_dto).context("response JSON serialization")?;
    println!("{pretty_json}");

    Ok(())
}

fn parse_socket_addr(tracker_socket_addr_str: &str) -> anyhow::Result<SocketAddr> {
    debug!("Tracker socket address: {tracker_socket_addr_str:#?}");

    // Check if the address is a valid URL. If so, extract the host and port.
    let resolved_addr = if let Ok(url) = Url::parse(tracker_socket_addr_str) {
        debug!("Tracker socket address URL: {url:?}");

        let host = url
            .host_str()
            .with_context(|| format!("invalid host in URL: `{tracker_socket_addr_str}`"))?
            .to_owned();

        let port = url
            .port()
            .with_context(|| format!("port not found in URL: `{tracker_socket_addr_str}`"))?
            .to_owned();

        (host, port)
    } else {
        // If not a URL, assume it's a host:port pair.

        let parts: Vec<&str> = tracker_socket_addr_str.split(':').collect();

        if parts.len() != 2 {
            return Err(anyhow::anyhow!(
                "invalid address format: `{}`. Expected format is host:port",
                tracker_socket_addr_str
            ));
        }

        let host = parts[0].to_owned();

        let port = parts[1]
            .parse::<u16>()
            .with_context(|| format!("invalid port: `{}`", parts[1]))?
            .to_owned();

        (host, port)
    };

    debug!("Resolved address: {resolved_addr:#?}");

    // Perform DNS resolution.
    let socket_addrs: Vec<_> = resolved_addr.to_socket_addrs()?.collect();
    if socket_addrs.is_empty() {
        Err(anyhow::anyhow!("DNS resolution failed for `{}`", tracker_socket_addr_str))
    } else {
        Ok(socket_addrs[0])
    }
}

fn parse_info_hash(info_hash_str: &str) -> anyhow::Result<TorrustInfoHash> {
    TorrustInfoHash::from_str(info_hash_str)
        .map_err(|e| anyhow::Error::msg(format!("failed to parse info-hash `{info_hash_str}`: {e:?}")))
}

// Define the ResponseDto enum to simplify the print_response function
#[derive(Serialize)]
enum ResponseDto {
    Connect(ConnectResponseDto),
    AnnounceIpv4(AnnounceResponseDto),
    AnnounceIpv6(AnnounceResponseDto),
    Scrape(ScrapeResponseDto),
    Error(ErrorResponseDto),
}

// Implement the From trait to convert from Response to ResponseDto
impl From<Response> for ResponseDto {
    fn from(response: Response) -> Self {
        match response {
            Response::Connect(res) => ResponseDto::Connect(ConnectResponseDto::from(res)),
            Response::AnnounceIpv4(res) => ResponseDto::AnnounceIpv4(AnnounceResponseDto::from(res)),
            Response::AnnounceIpv6(res) => ResponseDto::AnnounceIpv6(AnnounceResponseDto::from(res)),
            Response::Scrape(res) => ResponseDto::Scrape(ScrapeResponseDto::from(res)),
            Response::Error(res) => ResponseDto::Error(ErrorResponseDto::from(res)),
        }
    }
}

@mario-nt mario-nt marked this pull request as draft May 22, 2024 17:06
@mario-nt mario-nt force-pushed the 670-UDP-tracker-client-print-unexpected-responses-JSON branch from 1cd4756 to fe830c8 Compare May 27, 2024 13:59
@mario-nt mario-nt requested a review from josecelano May 27, 2024 15:44
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt this is much cleaner. I like it. In addition to the other suggestions I would suggest splitting the responses.rs mod into two mods:

  • responses.rs
    • mod.rs <- domain responses
    • json.rs <- serialized responses in JSON

use serde::Serialize;

pub trait DtoToJson {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt I think I would name this just ToJson.

Self: Serialize,
{
let pretty_json = serde_json::to_string_pretty(self).context("response JSON serialization")?;
println!("{pretty_json}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt I would remove the println from the trait. I would return a String. That would make it easier to test the implementations and you can use the method for other cases. Maybe you want to serialize to JSON but you don't want to print it in many cases.

@mario-nt mario-nt force-pushed the 670-UDP-tracker-client-print-unexpected-responses-JSON branch from 67a540f to 5a529cc Compare June 3, 2024 22:46
@mario-nt mario-nt marked this pull request as ready for review June 4, 2024 12:15
@mario-nt mario-nt requested a review from josecelano June 4, 2024 12:15
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt looks good!

@josecelano
Copy link
Member

ACK 0157d96

@josecelano josecelano merged commit 67ff5c4 into torrust:develop Jun 5, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Rebase Base Branch has Incompatibilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UDP Tracker client: Print unexpected responses in JSON
2 participants