-
Notifications
You must be signed in to change notification settings - Fork 44
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
[#670] new JSON serialization for connect and error aquatic responses #876
Conversation
Codecov ReportAttention: Patch coverage is
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. |
cff3d70
to
3c78be9
Compare
There was a problem hiding this 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)),
}
}
}
1cd4756
to
fe830c8
Compare
There was a problem hiding this 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
src/console/clients/udp/responses.rs
Outdated
use serde::Serialize; | ||
|
||
pub trait DtoToJson { |
There was a problem hiding this 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 I would name this just ToJson
.
src/console/clients/udp/responses.rs
Outdated
Self: Serialize, | ||
{ | ||
let pretty_json = serde_json::to_string_pretty(self).context("response JSON serialization")?; | ||
println!("{pretty_json}"); |
There was a problem hiding this 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 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.
…at and enum for Dto wrapper
…json serialization trait
67a540f
to
5a529cc
Compare
There was a problem hiding this 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!
ACK 0157d96 |
Resolves #670