From f7f29500d9cdb51b0e7acb4abce01a54fc6aa775 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 27 May 2026 19:11:18 +0100 Subject: [PATCH 1/2] refactor(http-protocol): decouple protocol types from domain primitives --- Cargo.lock | 2 +- ...eep_protocol_and_domain_types_decoupled.md | 92 ++++++++++++++++++ docs/adrs/index.md | 2 + .../open/1669-overhaul-packages/EPIC.md | 17 ++-- ...e-http-protocol-from-tracker-primitives.md | 94 +++++++++++++----- .../src/v1/extractors/announce_request.rs | 4 +- .../src/v1/handlers/announce.rs | 34 ++++++- .../src/v1/handlers/scrape.rs | 23 ++++- packages/http-protocol/Cargo.toml | 2 +- .../http-protocol/src/percent_encoding.rs | 31 ++++-- .../http-protocol/src/v1/requests/announce.rs | 72 +++++--------- .../src/v1/responses/announce.rs | 96 +++++++++++++------ .../http-protocol/src/v1/responses/scrape.rs | 38 ++++++-- .../http-tracker-core/benches/helpers/util.rs | 17 +++- .../src/services/announce.rs | 60 ++++++++++-- packages/udp-protocol/src/common.rs | 4 +- 16 files changed, 443 insertions(+), 145 deletions(-) create mode 100644 docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md diff --git a/Cargo.lock b/Cargo.lock index c7fcb4f8e..38d2a3d96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5331,6 +5331,7 @@ dependencies = [ name = "torrust-tracker-http-tracker-protocol" version = "3.0.0-develop" dependencies = [ + "bittorrent-peer-id", "bittorrent-primitives", "derive_more 2.1.1", "multimap", @@ -5341,7 +5342,6 @@ dependencies = [ "torrust-clock", "torrust-located-error", "torrust-tracker-contrib-bencode", - "torrust-tracker-primitives", ] [[package]] diff --git a/docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md b/docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md new file mode 100644 index 000000000..a20c6d54a --- /dev/null +++ b/docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md @@ -0,0 +1,92 @@ +--- +semantic-links: + skill-links: + - create-adr + related-artifacts: + - docs/issues/open/1669-overhaul-packages/EPIC.md + - docs/issues/open/1669-overhaul-packages/DECISIONS.md + - docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md + - docs/adrs/index.md + - packages/primitives/src/number_of_bytes.rs + - packages/http-protocol/src/v1/requests/announce.rs + - packages/udp-protocol/src/common.rs +--- + +# Keep Protocol And Domain Types Decoupled + +## Description + +Several value types currently exist in more than one package with similar field +shapes. A representative example is `NumberOfBytes`, which appears in: + +- `packages/primitives/src/number_of_bytes.rs` (domain-level meaning) +- `packages/http-protocol/src/v1/requests/announce.rs` (HTTP protocol DTO) +- `packages/udp-protocol/src/common.rs` (UDP protocol wire type) + +At first glance this can look like accidental duplication that should be +deduplicated into one shared type. However, these types live at different +architectural boundaries and have different reasons to change. + +The decision needed here is whether to enforce a single shared type across +layers/protocols, or to keep layer-local/protocol-local types and map at +boundaries. + +## Agreement + +Keep protocol and domain types decoupled, even when they share similar shape. + +This means: + +- Domain types remain domain-owned in `packages/primitives`. +- Protocol crates (`http-protocol`, `udp-protocol`) keep protocol-local types. +- Adapters perform explicit mapping at boundaries. + +This is an application of single-responsibility design: each layer has one +primary reason to change. + +- Domain types change when tracker domain/business policy changes. +- HTTP protocol types change when HTTP/BEP behavior or encoding constraints + change. +- UDP protocol types change when UDP/BEP behavior or wire representation + changes. + +As a consequence, a UDP wire-format change should not force broad domain +refactors, and a domain policy change should not force protocol crates to adopt +domain-centric shape. + +### Alternatives Considered + +**Single shared type for all layers/protocols** (for example one global +`NumberOfBytes` used by domain + HTTP + UDP). + +Rejected because: + +1. It couples protocol evolution to domain internals and vice versa. +2. It increases blast radius for protocol-specific changes. +3. It weakens boundary ownership and pushes cross-layer assumptions into shared + packages. + +### Consequences + +#### Positive + +- Clear boundaries and ownership per layer. +- Lower coupling between protocol evolution and tracker-domain evolution. +- Easier extraction/publication of protocol crates as independently evolving + packages. + +#### Negative + +- Some mapping code is required at adapter boundaries. +- Similar-looking structs may appear duplicated and require explicit + documentation to avoid accidental re-coupling. + +## Date + +2026-05-27 + +## References + +- EPIC: [docs/issues/open/1669-overhaul-packages/EPIC.md](../issues/open/1669-overhaul-packages/EPIC.md) +- Subissue SI-14: [docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md](../issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md) +- GitHub issue #1835: diff --git a/docs/adrs/index.md b/docs/adrs/index.md index ad12f76f8..92d95e748 100644 --- a/docs/adrs/index.md +++ b/docs/adrs/index.md @@ -5,6 +5,7 @@ semantic-links: related-artifacts: - docs/index.md - docs/adrs/README.md + - docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md --- # ADR Index @@ -16,6 +17,7 @@ semantic-links: | [20260429000000](20260429000000_keep_database_as_aggregate_supertrait.md) | 2026-04-29 | Keep `Database` as an aggregate supertrait | Split the 18-method monolithic `Database` trait into four narrow context traits (`SchemaMigrator`, `TorrentMetricsStore`, `WhitelistStore`, `AuthKeyStore`) while keeping `Database` as an empty aggregate supertrait with a blanket impl. | | [20260512102000](20260512102000_define_tracker_client_peer_id_convention.md) | 2026-05-12 | Define tracker-client peer ID convention | Adopt `-RC3000-` Azureus-style defaults for tracker-client, use a once-per-process randomized production suffix, and keep deterministic `RC` test fixtures without cross-package constant coupling. | | [20260519000000](20260519000000_define_global_cli_output_contract.md) | 2026-05-19 | Define the global CLI output contract | All first-party binaries use JSON on stdout (result data) and stderr (NDJSON diagnostics/progress). No plain text. TTY refusal for stdout-result-data commands. Exit codes 0/1/2. Prescriptive; migration is progressive. | +| [20260527175600](20260527175600_keep_protocol_and_domain_types_decoupled.md) | 2026-05-27 | Keep protocol and domain types decoupled | Keep protocol-local and domain-local value types (for example `NumberOfBytes`) and map at boundaries so HTTP/UDP wire evolution does not force domain-wide refactors and domain changes do not force protocol redesign. | ## ADR Lifecycle diff --git a/docs/issues/open/1669-overhaul-packages/EPIC.md b/docs/issues/open/1669-overhaul-packages/EPIC.md index d441aeabc..ebc8b2f0f 100644 --- a/docs/issues/open/1669-overhaul-packages/EPIC.md +++ b/docs/issues/open/1669-overhaul-packages/EPIC.md @@ -6,13 +6,16 @@ priority: p1 github-issue: 1669 spec-path: docs/issues/open/1669-overhaul-packages/EPIC.md epic-owner: josecelano -last-updated-utc: 2026-05-27 00:00 +last-updated-utc: 2026-05-27 18:00 semantic-links: skill-links: - create-issue related-artifacts: - docs/packages.md - docs/issues/open/1669-overhaul-packages/ + - docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md + - docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md + - docs/adrs/index.md - AGENTS.md --- @@ -239,8 +242,8 @@ The following crates remain in `torrust/torrust-tracker` for now: - `torrust-tracker-core` Rationale: current dependencies indicate unresolved layering/coupling. In particular, -`torrust-tracker-http-tracker-protocol` currently depends on -`torrust-tracker-primitives`. The move can be +`torrust-tracker-http-tracker-protocol` no longer depends on +`torrust-tracker-primitives` (completed in SI-14, #1835). The move can be revisited after these dependencies are clarified and reduced. > **Naming policy**: prefix reflects ownership and release identity, not estimated @@ -351,7 +354,6 @@ This section lists direct crate dependencies that have a `torrust*` prefix. - `torrust-bencode` - `torrust-clock` - `torrust-located-error` - - `torrust-tracker-primitives` - `torrust-tracker-udp-tracker-core` - `torrust-clock` - `torrust-metrics` @@ -534,7 +536,7 @@ Status: TODO unless noted. #### 3. Numbered Subissues (GitHub Issues Open) - [x] [#1834](https://github.com/torrust/torrust-tracker/issues/1834) SI-13: Decouple `http-protocol` from `udp-protocol` _(Rule M; remove cross-protocol dependency edge)_ -- [ ] [#1835](https://github.com/torrust/torrust-tracker/issues/1835) SI-14: Decouple `http-protocol` from `torrust-tracker-primitives` _(Rule M; remove protocol -> domain coupling as step 2)_ +- [x] [#1835](https://github.com/torrust/torrust-tracker/issues/1835) SI-14: Decouple `http-protocol` from `torrust-tracker-primitives` _(Rule M; remove protocol -> domain coupling as step 2)_ #### 4. Draft Specs (No Subissue Number, No GitHub Issue) @@ -571,7 +573,10 @@ Details: | Rename-to-desired-state | [#1829](https://github.com/torrust/torrust-tracker/issues/1829) — Rename crates and folder names to match desired `torrust-tracker` workspace state | [docs/issues/closed/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md](../../closed/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md) | DONE | SI-11 complete; spec archived to `docs/issues/closed/` after issue closure | | HTTP protocol decoupling | [#1830](https://github.com/torrust/torrust-tracker/issues/1830) — Decouple `http-protocol` from `tracker-core` | [docs/issues/closed/1830-1669-12-decouple-http-protocol-from-tracker-core.md](../../closed/1830-1669-12-decouple-http-protocol-from-tracker-core.md) | DONE | SI-12 complete; removed `http-protocol -> tracker-core` edge and moved mapping to higher layer | | HTTP/UDP decoupling | [#1834](https://github.com/torrust/torrust-tracker/issues/1834) — Decouple `http-protocol` from `udp-protocol` | [docs/issues/open/1834-1669-13-decouple-http-protocol-from-udp-protocol.md](../../open/1834-1669-13-decouple-http-protocol-from-udp-protocol.md) | DONE | SI-13 complete; removed `http-protocol -> udp-protocol` edge | -| HTTP/primitives decoupling | [#1835](https://github.com/torrust/torrust-tracker/issues/1835) — Decouple `http-protocol` from `torrust-tracker-primitives` | [docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md](../../open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md) | TODO | SI-14. Rule M; execute after SI-13; remove protocol -> domain coupling in step 2 | +| HTTP/primitives decoupling | [#1835](https://github.com/torrust/torrust-tracker/issues/1835) — Decouple `http-protocol` from `torrust-tracker-primitives` | [docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md](../../open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md) | DONE | SI-14 complete; protocol-owned DTOs introduced and boundary mapping moved to core/server layers | + +Proposal note: +After SI-14, there is a proposal to evaluate a dedicated repository for protocol crates so protocol packages can evolve with BEP/spec changes while tracker app packages evolve with domain/product changes. This is proposal-only for now (not committed scope) and is tracked in [#1835](https://github.com/torrust/torrust-tracker/issues/1835) and [docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md](../../open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md). ### Draft issues diff --git a/docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md b/docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md index 234552691..a96f72de8 100644 --- a/docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md +++ b/docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md @@ -1,23 +1,30 @@ --- doc-type: issue issue-type: task -status: planned +status: in_progress priority: p1 github-issue: 1835 spec-path: docs/issues/open/1835-1669-14-decouple-http-protocol-from-tracker-primitives.md -branch: null +branch: 1835-1669-14-decouple-http-protocol-from-tracker-primitives related-pr: null -last-updated-utc: 2026-05-27 00:00 +last-updated-utc: 2026-05-27 18:00 semantic-links: skill-links: - create-issue related-artifacts: - docs/issues/open/1669-overhaul-packages/EPIC.md + - docs/issues/open/1669-overhaul-packages/DECISIONS.md + - docs/adrs/20260527175600_keep_protocol_and_domain_types_decoupled.md - packages/http-protocol/Cargo.toml - packages/http-protocol/src/v1/requests/announce.rs + - packages/http-protocol/src/v1/responses/announce.rs + - packages/http-protocol/src/v1/responses/scrape.rs - packages/primitives/src/announce.rs + - packages/primitives/src/number_of_bytes.rs + - packages/udp-protocol/src/common.rs - packages/http-tracker-core/src/services/announce.rs - packages/axum-http-server/src/v1/handlers/announce.rs + - packages/axum-http-server/src/v1/handlers/scrape.rs --- @@ -108,28 +115,28 @@ Symbol-level usage inside protocol: Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Confirm all `torrust-tracker-primitives` usages in `http-protocol` and document symbol-level evidence | Evidence captured in PR description | -| T2 | TODO | Remove direct primitive conversion impls from `packages/http-protocol/src/v1/requests/announce.rs` | No direct `torrust_tracker_primitives::` references remain in source | -| T3 | TODO | Remove `torrust-tracker-primitives` from `packages/http-protocol/Cargo.toml` | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` shows no edge | -| T4 | TODO | Add/adjust mapping in higher layers (`http-tracker-core` as primary owner; `axum-http-server` only if needed) | Event behavior remains equivalent | -| T5 | TODO | Update tests and fixtures | Tests compile and pass without direct protocol->domain coupling | -| T6 | TODO | Run verification commands | Build/tests/lints pass | -| T7 | TODO | Update EPIC tracking rows and draft list as needed | Active Subissues remain consistent | -| T8 | TODO | Update EPIC after implementation | Update Active Subissues progress and EPIC sections: Package Inventory, Desired Package State, Torrust Dependency Lists (Direct, Non-dev) | +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ | +| T1 | DONE | Confirm all `torrust-tracker-primitives` usages in `http-protocol` and document symbol-level evidence | Evidence captured via `rg` and `cargo tree` outputs | +| T2 | DONE | Remove direct primitive conversion impls from `packages/http-protocol/src/v1/requests/announce.rs` | No direct `torrust_tracker_primitives::` references remain in source | +| T3 | DONE | Remove `torrust-tracker-primitives` from `packages/http-protocol/Cargo.toml` | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` shows no edge | +| T4 | DONE | Add/adjust mapping in higher layers (`http-tracker-core` as primary owner; `axum-http-server` only if needed) | Event mapping now lives in `http-tracker-core`; response DTO mapping lives in `axum-http-server` | +| T5 | DONE | Update tests and fixtures | Protocol/core/server tests and benchmark fixtures updated | +| T6 | DONE | Run verification commands | Build/tests/lints pass | +| T7 | DONE | Update EPIC tracking rows and draft list as needed | Active Subissues row updated | +| T8 | DONE | Update EPIC after implementation | EPIC dependency notes updated for `http-protocol` | ## Acceptance Criteria -- [ ] `packages/http-protocol/Cargo.toml` has no `torrust-tracker-primitives` dependency. -- [ ] `packages/http-protocol` has no source-level references to +- [x] `packages/http-protocol/Cargo.toml` has no `torrust-tracker-primitives` dependency. +- [x] `packages/http-protocol` has no source-level references to `torrust_tracker_primitives::`. -- [ ] HTTP announce event behavior remains unchanged for +- [x] HTTP announce event behavior remains unchanged for `started/stopped/completed/none` mappings. -- [ ] `cargo build --workspace` passes. -- [ ] Relevant tests in HTTP protocol/core/server packages pass. -- [ ] `linter all` exits with code `0`. -- [ ] EPIC tracking includes this subissue. +- [x] `cargo build --workspace` passes. +- [x] Relevant tests in HTTP protocol/core/server packages pass. +- [x] `linter all` exits with code `0`. +- [x] EPIC tracking includes this subissue. ## Verification Plan @@ -145,11 +152,11 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. -| ID | Scenario | Command / Steps | Expected Result | Status | Evidence | -| --- | ---------------------------------------- | --------------------------------------------------------------- | ---------------------------------------------------------- | ------ | -------- | -| M1 | No protocol->domain edge remains | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` | No dependency on `torrust-tracker-primitives` | TODO | | -| M2 | No primitives symbols in protocol source | `rg "torrust_tracker_primitives::" packages/http-protocol` | No matches | TODO | | -| M3 | Event conversion behavior preserved | Run existing announce request parsing/unit tests | Mappings for `started/stopped/completed/none` stay correct | TODO | | +| ID | Scenario | Command / Steps | Expected Result | Status | Evidence | +| --- | ---------------------------------------- | --------------------------------------------------------------- | ---------------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| M1 | No protocol->domain edge remains | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` | No dependency on `torrust-tracker-primitives` | DONE | Output shows `bittorrent-peer-id` and no `torrust-tracker-primitives` | +| M2 | No primitives symbols in protocol source | `rg "torrust_tracker_primitives::" packages/http-protocol` | No matches | DONE | No matches returned | +| M3 | Event conversion behavior preserved | Run existing announce request parsing/unit tests | Mappings for `started/stopped/completed/none` stay correct | DONE | `cargo test -p torrust-tracker-http-tracker-protocol`, `cargo test -p torrust-tracker-http-tracker-core`, and `cargo test -p torrust-tracker-axum-http-server` passed | ## Risks and Trade-offs @@ -157,6 +164,43 @@ Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. clear and avoid duplicate conversion logic. - Temporary compatibility helpers may be needed while call sites migrate. +## Post-Implementation Reasoning (Intentional Duplication) + +The implementation introduces protocol-local DTOs that can look similar to +domain types (for example `SwarmMetadata` and `ScrapeData`). This duplication +is intentional and preserves a clean layering boundary: + +- Protocol crates model BEP/wire semantics and should evolve with protocol + changes. +- Similar concepts may also appear across protocol crates (for example + `NumberOfBytes` in HTTP and UDP). This inter-protocol duplication is also + intentional so one protocol can change wire representation/constraints + without forcing synchronized changes in other protocols. +- Tracker/domain crates model application semantics and should evolve with + tracker policy and product decisions. +- Boundary adapters (`http-tracker-core` and `axum-http-server`) absorb + translation costs and prevent protocol-change blast radius across the app. + +Trade-off acknowledgement: + +- There is a small conversion overhead at boundaries. +- In exchange, coupling is reduced and protocol/domain life cycles stay + independent. + +This is aligned with DEC-06 and is preferred over re-coupling higher layers to +protocol DTOs. + +## Follow-up Proposal + +Consider extracting protocol crates to a dedicated protocol-focused repository +in a future EPIC phase. This would make lifecycle boundaries explicit: + +- Protocol crates evolve with BEP/spec evolution. +- Tracker application crates evolve with product/domain evolution. + +This subissue does not perform that extraction; it only prepares for it by +removing protocol -> domain coupling. + ## References - EPIC: [docs/issues/open/1669-overhaul-packages/EPIC.md](1669-overhaul-packages/EPIC.md) diff --git a/packages/axum-http-server/src/v1/extractors/announce_request.rs b/packages/axum-http-server/src/v1/extractors/announce_request.rs index 6c387781b..812f3fba1 100644 --- a/packages/axum-http-server/src/v1/extractors/announce_request.rs +++ b/packages/axum-http-server/src/v1/extractors/announce_request.rs @@ -87,9 +87,9 @@ mod tests { use std::str::FromStr; use bittorrent_primitives::info_hash::InfoHash; - use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event}; + use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event, NumberOfBytes}; use torrust_tracker_http_tracker_protocol::v1::responses::error::Error; - use torrust_tracker_primitives::{NumberOfBytes, PeerId}; + use torrust_tracker_primitives::PeerId; use super::extract_announce_from; diff --git a/packages/axum-http-server/src/v1/handlers/announce.rs b/packages/axum-http-server/src/v1/handlers/announce.rs index 51ead5158..f09b923ac 100644 --- a/packages/axum-http-server/src/v1/handlers/announce.rs +++ b/packages/axum-http-server/src/v1/handlers/announce.rs @@ -13,7 +13,7 @@ use torrust_tracker_http_tracker_core::services::announce::{AnnounceService, Htt use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, Compact}; use torrust_tracker_http_tracker_protocol::v1::responses::{self}; use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources; -use torrust_tracker_primitives::AnnounceData; +use torrust_tracker_primitives::AnnounceData as DomainAnnounceData; use crate::v1::extractors::announce_request::ExtractRequest; use crate::v1::extractors::authentication_key::Extract as ExtractKey; @@ -81,24 +81,48 @@ async fn handle_announce( client_ip_sources: &ClientIpSources, server_service_binding: &ServiceBinding, maybe_key: Option, -) -> Result { +) -> Result { announce_service .handle_announce(announce_request, client_ip_sources, server_service_binding, maybe_key) .await } -fn build_response(announce_request: &Announce, announce_data: AnnounceData) -> Response { +fn build_response(announce_request: &Announce, announce_data: DomainAnnounceData) -> Response { + let protocol_data = to_protocol_announce_data(announce_data); + if announce_request.compact.as_ref().is_some_and(|f| *f == Compact::Accepted) { - let response: responses::Announce = announce_data.into(); + let response: responses::Announce = protocol_data.into(); let bytes: Vec = response.data.into(); (StatusCode::OK, bytes).into_response() } else { - let response: responses::Announce = announce_data.into(); + let response: responses::Announce = protocol_data.into(); let bytes: Vec = response.data.into(); (StatusCode::OK, bytes).into_response() } } +fn to_protocol_announce_data(domain_data: DomainAnnounceData) -> responses::announce::AnnounceData { + responses::announce::AnnounceData { + peers: domain_data + .peers + .into_iter() + .map(|peer| responses::announce::Peer { + peer_id: peer.peer_id, + peer_addr: peer.peer_addr, + }) + .collect(), + stats: responses::announce::SwarmMetadata { + complete: domain_data.stats.complete, + downloaded: domain_data.stats.downloaded, + incomplete: domain_data.stats.incomplete, + }, + policy: responses::announce::AnnouncePolicy { + interval: domain_data.policy.interval, + interval_min: domain_data.policy.interval_min, + }, + } +} + #[cfg(test)] mod tests { diff --git a/packages/axum-http-server/src/v1/handlers/scrape.rs b/packages/axum-http-server/src/v1/handlers/scrape.rs index 92cbcb81f..d70eaaaca 100644 --- a/packages/axum-http-server/src/v1/handlers/scrape.rs +++ b/packages/axum-http-server/src/v1/handlers/scrape.rs @@ -13,7 +13,7 @@ use torrust_tracker_http_tracker_core::services::scrape::ScrapeService; use torrust_tracker_http_tracker_protocol::v1::requests::scrape::Scrape; use torrust_tracker_http_tracker_protocol::v1::responses; use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources; -use torrust_tracker_primitives::ScrapeData; +use torrust_tracker_primitives::ScrapeData as DomainScrapeData; use crate::v1::extractors::authentication_key::Extract as ExtractKey; use crate::v1::extractors::client_ip_sources::Extract as ExtractClientIpSources; @@ -69,12 +69,29 @@ async fn handle( build_response(scrape_data) } -fn build_response(scrape_data: ScrapeData) -> Response { - let response = responses::scrape::Bencoded::from(scrape_data); +fn build_response(scrape_data: DomainScrapeData) -> Response { + let response = responses::scrape::Bencoded::from(to_protocol_scrape_data(scrape_data)); (StatusCode::OK, response.body()).into_response() } +fn to_protocol_scrape_data(domain_data: DomainScrapeData) -> responses::scrape::ScrapeData { + let mut protocol_data = responses::scrape::ScrapeData::empty(); + + for (info_hash, metadata) in domain_data.files { + protocol_data.add_file( + &info_hash, + responses::scrape::SwarmMetadata { + complete: metadata.complete, + downloaded: metadata.downloaded, + incomplete: metadata.incomplete, + }, + ); + } + + protocol_data +} + #[cfg(test)] mod tests { use std::net::{IpAddr, Ipv4Addr, SocketAddr}; diff --git a/packages/http-protocol/Cargo.toml b/packages/http-protocol/Cargo.toml index 617aea054..05d11af0e 100644 --- a/packages/http-protocol/Cargo.toml +++ b/packages/http-protocol/Cargo.toml @@ -16,6 +16,7 @@ version.workspace = true [dependencies] bittorrent-primitives = "0.2.0" +bittorrent-peer-id = { version = "3.0.0-develop", path = "../peer-id" } derive_more = { version = "2", features = [ "as_ref", "constructor", "from" ] } multimap = "0" percent-encoding = "2" @@ -25,4 +26,3 @@ thiserror = "2" torrust-clock = { version = "3.0.0-develop", path = "../clock" } torrust-tracker-contrib-bencode = { version = "3.0.0-develop", path = "../../contrib/bencode" } torrust-located-error = { version = "3.0.0-develop", path = "../located-error" } -torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" } diff --git a/packages/http-protocol/src/percent_encoding.rs b/packages/http-protocol/src/percent_encoding.rs index 2b5d1ceac..cee11bf08 100644 --- a/packages/http-protocol/src/percent_encoding.rs +++ b/packages/http-protocol/src/percent_encoding.rs @@ -15,8 +15,16 @@ //! - //! - //! - +use bittorrent_peer_id::PeerId; use bittorrent_primitives::info_hash::{self, InfoHash}; -use torrust_tracker_primitives::{PeerId, peer}; + +#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)] +pub enum PeerIdConversionError { + #[error("Peer id too short: expected 20 bytes, got {actual}")] + NotEnoughBytes { actual: usize }, + #[error("Peer id too long: expected 20 bytes, got {actual}")] + TooManyBytes { actual: usize }, +} /// Percent decodes a percent encoded infohash. Internally an /// [`InfoHash`] is a 20-byte array. @@ -28,7 +36,6 @@ use torrust_tracker_primitives::{PeerId, peer}; /// use std::str::FromStr; /// use torrust_tracker_http_tracker_protocol::percent_encoding::percent_decode_info_hash; /// use bittorrent_primitives::info_hash::InfoHash; -/// use torrust_tracker_primitives::peer; /// /// let encoded_infohash = "%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0"; /// @@ -60,7 +67,7 @@ pub fn percent_decode_info_hash(raw_info_hash: &str) -> Result Result Result { +pub fn percent_decode_peer_id(raw_peer_id: &str) -> Result { let bytes = percent_encoding::percent_decode_str(raw_peer_id).collect::>(); - Ok(*peer::Id::try_from(bytes)?) + + if bytes.len() < 20 { + return Err(PeerIdConversionError::NotEnoughBytes { actual: bytes.len() }); + } + + if bytes.len() > 20 { + return Err(PeerIdConversionError::TooManyBytes { actual: bytes.len() }); + } + + let mut peer_id = [0_u8; 20]; + peer_id.copy_from_slice(&bytes); + + Ok(PeerId(peer_id)) } #[cfg(test)] mod tests { use std::str::FromStr; + use bittorrent_peer_id::PeerId; use bittorrent_primitives::info_hash::InfoHash; - use torrust_tracker_primitives::PeerId; use crate::percent_encoding::{percent_decode_info_hash, percent_decode_peer_id}; diff --git a/packages/http-protocol/src/v1/requests/announce.rs b/packages/http-protocol/src/v1/requests/announce.rs index 7b2954426..b895e027c 100644 --- a/packages/http-protocol/src/v1/requests/announce.rs +++ b/packages/http-protocol/src/v1/requests/announce.rs @@ -2,18 +2,15 @@ //! //! Data structures and logic for parsing the `announce` request. use std::fmt; -use std::net::{IpAddr, SocketAddr}; use std::panic::Location; use std::str::FromStr; +use bittorrent_peer_id::PeerId; use bittorrent_primitives::info_hash::{self, InfoHash}; use thiserror::Error; -use torrust_clock::clock::Time; use torrust_located_error::{Located, LocatedError}; -use torrust_tracker_primitives::{AnnounceEvent, NumberOfBytes, PeerId, peer}; -use crate::CurrentClock; -use crate::percent_encoding::{percent_decode_info_hash, percent_decode_peer_id}; +use crate::percent_encoding::{PeerIdConversionError, percent_decode_info_hash, percent_decode_peer_id}; use crate::v1::query::{ParseQueryError, Query}; use crate::v1::responses; @@ -28,13 +25,28 @@ const EVENT: &str = "event"; const COMPACT: &str = "compact"; const NUMWANT: &str = "numwant"; +// Intentionally protocol-local: this currently mirrors the UDP protocol +// `NumberOfBytes` concept and domain byte counters, but it is kept local so +// HTTP wire semantics can evolve independently without forcing cross-protocol +// or domain-wide refactors. +#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)] +pub struct NumberOfBytes(pub i64); + +impl NumberOfBytes { + #[must_use] + pub const fn new(v: i64) -> Self { + Self(v) + } +} + /// The `Announce` request. Fields use the domain types after parsing the /// query params of the request. /// /// ```rust /// use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event}; /// use bittorrent_primitives::info_hash::InfoHash; -/// use torrust_tracker_primitives::{NumberOfBytes, PeerId}; +/// use bittorrent_peer_id::PeerId; +/// use torrust_tracker_http_tracker_protocol::v1::requests::announce::NumberOfBytes; /// /// let request = Announce { /// // Mandatory params @@ -133,7 +145,7 @@ pub enum ParseAnnounceQueryError { InvalidPeerIdParam { param_name: String, param_value: String, - source: LocatedError<'static, peer::IdConversionError>, + source: LocatedError<'static, PeerIdConversionError>, }, } @@ -190,28 +202,6 @@ impl fmt::Display for Event { } } -impl From for Event { - fn from(event: AnnounceEvent) -> Self { - match event { - AnnounceEvent::Started => Self::Started, - AnnounceEvent::Stopped => Self::Stopped, - AnnounceEvent::Completed => Self::Completed, - AnnounceEvent::None => Self::Empty, - } - } -} - -impl From for AnnounceEvent { - fn from(event: Event) -> Self { - match event { - Event::Started => Self::Started, - Event::Stopped => Self::Stopped, - Event::Completed => Self::Completed, - Event::Empty => Self::None, - } - } -} - /// Whether the `announce` response should be in compact mode or not. /// /// Depending on the value of this param, the tracker will return a different @@ -405,36 +395,18 @@ fn extract_numwant(query: &Query) -> Result, ParseAnnounceQueryError } } -/// It builds a `Peer` from the announce request. -/// -/// It ignores the peer address in the announce request params. -#[must_use] -pub fn peer_from_request(announce_request: &Announce, peer_ip: &IpAddr) -> peer::Peer { - peer::Peer { - peer_id: announce_request.peer_id, - peer_addr: SocketAddr::new(*peer_ip, announce_request.port), - updated: CurrentClock::now(), - uploaded: announce_request.uploaded.unwrap_or(NumberOfBytes::new(0)), - downloaded: announce_request.downloaded.unwrap_or(NumberOfBytes::new(0)), - left: announce_request.left.unwrap_or(NumberOfBytes::new(0)), - event: match &announce_request.event { - Some(event) => event.clone().into(), - None => AnnounceEvent::None, - }, - } -} - #[cfg(test)] mod tests { mod announce_request { + use bittorrent_peer_id::PeerId; use bittorrent_primitives::info_hash::InfoHash; - use torrust_tracker_primitives::{NumberOfBytes, PeerId}; use crate::v1::query::Query; use crate::v1::requests::announce::{ - Announce, COMPACT, Compact, DOWNLOADED, EVENT, Event, INFO_HASH, LEFT, NUMWANT, PEER_ID, PORT, UPLOADED, + Announce, COMPACT, Compact, DOWNLOADED, EVENT, Event, INFO_HASH, LEFT, NUMWANT, NumberOfBytes, PEER_ID, PORT, + UPLOADED, }; #[test] diff --git a/packages/http-protocol/src/v1/responses/announce.rs b/packages/http-protocol/src/v1/responses/announce.rs index cbf4f734c..f545f8cab 100644 --- a/packages/http-protocol/src/v1/responses/announce.rs +++ b/packages/http-protocol/src/v1/responses/announce.rs @@ -2,11 +2,60 @@ //! //! Data structures and logic to build the `announce` response. use std::io::Write; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use bittorrent_peer_id::PeerId; use derive_more::{AsRef, Constructor, From}; use torrust_tracker_contrib_bencode::{BMutAccess, BencodeMut, ben_bytes, ben_int, ben_list, ben_map}; -use torrust_tracker_primitives::{AnnounceData, peer}; + +// Protocol-local announce response DTOs intentionally duplicate some domain +// field shapes. This keeps protocol crates decoupled from tracker domain types +// and centralizes conversions in boundary adapters. +#[derive(Clone, Debug, PartialEq, Constructor, Default)] +pub struct AnnounceData { + pub peers: Vec, + pub stats: SwarmMetadata, + pub policy: AnnouncePolicy, +} + +#[derive(PartialEq, Eq, Debug, Clone, Copy, Constructor)] +pub struct AnnouncePolicy { + pub interval: u32, + pub interval_min: u32, +} + +impl Default for AnnouncePolicy { + fn default() -> Self { + Self { + interval: 120, + interval_min: 120, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub struct SwarmMetadata { + pub complete: u32, + pub downloaded: u32, + pub incomplete: u32, +} + +impl SwarmMetadata { + #[must_use] + pub const fn new(complete: u32, downloaded: u32, incomplete: u32) -> Self { + Self { + complete, + downloaded, + incomplete, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct Peer { + pub peer_id: PeerId, + pub peer_addr: SocketAddr, +} /// An [`Announce`] response, that can be anything that is convertible from [`AnnounceData`]. /// @@ -56,7 +105,7 @@ impl From for Normal { incomplete: data.stats.incomplete.into(), interval: data.policy.interval.into(), min_interval: data.policy.interval_min.into(), - peers: data.peers.iter().map(AsRef::as_ref).copied().collect(), + peers: data.peers.into_iter().map(NormalPeer::from).collect(), } } } @@ -93,7 +142,7 @@ pub struct Compact { impl From for Compact { fn from(data: AnnounceData) -> Self { - let compact_peers: Vec = data.peers.iter().map(AsRef::as_ref).copied().collect(); + let compact_peers: Vec = data.peers.into_iter().map(CompactPeer::from).collect(); let (peers, peers6): (Vec>, Vec>) = compact_peers.into_iter().collect(); @@ -150,10 +199,8 @@ pub struct NormalPeer { pub port: u16, } -impl peer::Encoding for NormalPeer {} - -impl From for NormalPeer { - fn from(peer: peer::Peer) -> Self { +impl From for NormalPeer { + fn from(peer: Peer) -> Self { NormalPeer { peer_id: peer.peer_id.0, ip: peer.peer_addr.ip(), @@ -202,10 +249,8 @@ pub enum CompactPeer { V6(CompactPeerData), } -impl peer::Encoding for CompactPeer {} - -impl From for CompactPeer { - fn from(peer: peer::Peer) -> Self { +impl From for CompactPeer { + fn from(peer: Peer) -> Self { match (peer.peer_addr.ip(), peer.peer_addr.port()) { (IpAddr::V4(ip), port) => Self::V4(CompactPeerData { ip, port }), (IpAddr::V6(ip), port) => Self::V6(CompactPeerData { ip, port }), @@ -275,13 +320,10 @@ impl FromIterator> for CompactPeersEncoded { mod tests { use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; - use std::sync::Arc; - use torrust_tracker_primitives::peer::fixture::PeerBuilder; - use torrust_tracker_primitives::swarm_metadata::SwarmMetadata; - use torrust_tracker_primitives::{AnnounceData, AnnouncePolicy, PeerId}; + use bittorrent_peer_id::PeerId; - use crate::v1::responses::announce::{Announce, Compact, Normal}; + use crate::v1::responses::announce::{Announce, AnnounceData, AnnouncePolicy, Compact, Normal, Peer, SwarmMetadata}; // Some ascii values used in tests: // @@ -298,20 +340,20 @@ mod tests { fn setup_announce_data() -> AnnounceData { let policy = AnnouncePolicy::new(111, 222); - let peer_ipv4 = PeerBuilder::default() - .with_peer_id(&PeerId(*b"-RC3000-000000000001")) - .with_peer_addr(&SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0x69, 0x69, 0x69, 0x69)), 0x7070)) - .build(); + let peer_ipv4 = Peer { + peer_id: PeerId(*b"-RC3000-000000000001"), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0x69, 0x69, 0x69, 0x69)), 0x7070), + }; - let peer_ipv6 = PeerBuilder::default() - .with_peer_id(&PeerId(*b"-RC3000-000000000002")) - .with_peer_addr(&SocketAddr::new( + let peer_ipv6 = Peer { + peer_id: PeerId(*b"-RC3000-000000000002"), + peer_addr: SocketAddr::new( IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969)), 0x7070, - )) - .build(); + ), + }; - let peers = vec![Arc::new(peer_ipv4), Arc::new(peer_ipv6)]; + let peers = vec![peer_ipv4, peer_ipv6]; let stats = SwarmMetadata::new(333, 333, 444); AnnounceData::new(peers, stats, policy) diff --git a/packages/http-protocol/src/v1/responses/scrape.rs b/packages/http-protocol/src/v1/responses/scrape.rs index fd8b06c7c..7d2bfd988 100644 --- a/packages/http-protocol/src/v1/responses/scrape.rs +++ b/packages/http-protocol/src/v1/responses/scrape.rs @@ -2,17 +2,45 @@ //! //! Data structures and logic to build the `scrape` response. use std::borrow::Cow; +use std::collections::BTreeMap; +use bittorrent_primitives::info_hash::InfoHash; use torrust_tracker_contrib_bencode::{BMutAccess, ben_int, ben_map}; -use torrust_tracker_primitives::ScrapeData; + +// These protocol DTOs intentionally mirror some domain fields but must remain +// protocol-owned. Keeping this type local avoids protocol->domain coupling and +// confines translation to boundary adapters. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub struct SwarmMetadata { + pub complete: u32, + pub downloaded: u32, + pub incomplete: u32, +} + +// Intentional boundary duplication: this represents scrape response payload +// semantics for the HTTP protocol crate, not tracker-domain semantics. +#[derive(Clone, Debug, PartialEq, Default)] +pub struct ScrapeData { + pub files: BTreeMap, +} + +impl ScrapeData { + #[must_use] + pub fn empty() -> Self { + Self::default() + } + + pub fn add_file(&mut self, info_hash: &InfoHash, swarm_metadata: SwarmMetadata) { + self.files.insert(*info_hash, swarm_metadata); + } +} /// The `Scrape` response for the HTTP tracker. /// /// ```rust /// use torrust_tracker_http_tracker_protocol::v1::responses::scrape::Bencoded; /// use bittorrent_primitives::info_hash::InfoHash; -/// use torrust_tracker_primitives::swarm_metadata::SwarmMetadata; -/// use torrust_tracker_primitives::ScrapeData; +/// use torrust_tracker_http_tracker_protocol::v1::responses::scrape::{ScrapeData, SwarmMetadata}; /// /// let info_hash = InfoHash::from_bytes(&[0x69; 20]); /// let mut scrape_data = ScrapeData::empty(); @@ -84,10 +112,8 @@ mod tests { mod scrape_response { use bittorrent_primitives::info_hash::InfoHash; - use torrust_tracker_primitives::ScrapeData; - use torrust_tracker_primitives::swarm_metadata::SwarmMetadata; - use crate::v1::responses::scrape::Bencoded; + use crate::v1::responses::scrape::{Bencoded, ScrapeData, SwarmMetadata}; fn sample_scrape_data() -> ScrapeData { let info_hash = InfoHash::from_bytes(&[0x69; 20]); diff --git a/packages/http-tracker-core/benches/helpers/util.rs b/packages/http-tracker-core/benches/helpers/util.rs index 338f6ba78..5698eed36 100644 --- a/packages/http-tracker-core/benches/helpers/util.rs +++ b/packages/http-tracker-core/benches/helpers/util.rs @@ -21,7 +21,9 @@ use torrust_tracker_http_tracker_core::event::bus::EventBus; use torrust_tracker_http_tracker_core::event::sender::Broadcaster; use torrust_tracker_http_tracker_core::statistics::event::listener::run_event_listener; use torrust_tracker_http_tracker_core::statistics::repository::Repository; -use torrust_tracker_http_tracker_protocol::v1::requests::announce::Announce; +use torrust_tracker_http_tracker_protocol::v1::requests::announce::{ + Announce, Event as ProtocolAnnounceEvent, NumberOfBytes as ProtocolNumberOfBytes, +}; use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources; use torrust_tracker_primitives::peer::Peer; use torrust_tracker_primitives::{AnnounceEvent, NumberOfBytes, PeerId, peer}; @@ -105,10 +107,15 @@ pub fn sample_announce_request_for_peer(peer: Peer) -> (Announce, ClientIpSource info_hash: sample_info_hash(), peer_id: peer.peer_id, port: peer.peer_addr.port(), - uploaded: Some(peer.uploaded), - downloaded: Some(peer.downloaded), - left: Some(peer.left), - event: Some(peer.event.into()), + uploaded: Some(ProtocolNumberOfBytes::new(peer.uploaded.0)), + downloaded: Some(ProtocolNumberOfBytes::new(peer.downloaded.0)), + left: Some(ProtocolNumberOfBytes::new(peer.left.0)), + event: Some(match peer.event { + AnnounceEvent::Started => ProtocolAnnounceEvent::Started, + AnnounceEvent::Stopped => ProtocolAnnounceEvent::Stopped, + AnnounceEvent::Completed => ProtocolAnnounceEvent::Completed, + AnnounceEvent::None => ProtocolAnnounceEvent::Empty, + }), compact: None, numwant: None, }; diff --git a/packages/http-tracker-core/src/services/announce.rs b/packages/http-tracker-core/src/services/announce.rs index 65ccf12bd..476de159c 100644 --- a/packages/http-tracker-core/src/services/announce.rs +++ b/packages/http-tracker-core/src/services/announce.rs @@ -11,6 +11,7 @@ use std::panic::Location; use std::sync::Arc; use bittorrent_primitives::info_hash::InfoHash; +use torrust_clock::clock::Time; use torrust_net_primitives::service_binding::ServiceBinding; use torrust_tracker_configuration::Core; use torrust_tracker_core::announce_handler::{AnnounceHandler, PeersWanted}; @@ -18,13 +19,15 @@ use torrust_tracker_core::authentication::service::AuthenticationService; use torrust_tracker_core::authentication::{self, Key}; use torrust_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistError}; use torrust_tracker_core::whitelist; -use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, peer_from_request}; +use torrust_tracker_http_tracker_protocol::v1::requests::announce::{ + Announce, Event as ProtocolAnnounceEvent, NumberOfBytes as ProtocolNumberOfBytes, +}; use torrust_tracker_http_tracker_protocol::v1::responses::error::Error as HttpProtocolErrorResponse; use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::{ ClientIpSources, PeerIpResolutionError, RemoteClientAddr, resolve_remote_client_addr, }; -use torrust_tracker_primitives::AnnounceData; use torrust_tracker_primitives::peer::PeerAnnouncement; +use torrust_tracker_primitives::{AnnounceData, AnnounceEvent, NumberOfBytes}; use crate::event; use crate::event::Event; @@ -83,7 +86,7 @@ impl AnnounceService { let remote_client_addr = resolve_remote_client_addr(&self.core_config.net.on_reverse_proxy.into(), client_ip_sources)?; - let mut peer = peer_from_request(announce_request, &remote_client_addr.ip()); + let mut peer = Self::peer_from_request(announce_request, &remote_client_addr.ip()); let peers_wanted = Self::peers_wanted(announce_request); @@ -108,6 +111,34 @@ impl AnnounceService { Ok(announce_data) } + fn peer_from_request(announce_request: &Announce, peer_ip: &std::net::IpAddr) -> PeerAnnouncement { + // Intentional adapter boundary: map protocol-owned request DTOs into + // domain announcements here instead of sharing domain types with the + // protocol crate. This limits coupling and keeps protocol evolution + // from forcing domain-wide refactors. + let uploaded = announce_request.uploaded.unwrap_or(ProtocolNumberOfBytes::new(0)); + let downloaded = announce_request.downloaded.unwrap_or(ProtocolNumberOfBytes::new(0)); + let left = announce_request.left.unwrap_or(ProtocolNumberOfBytes::new(0)); + + PeerAnnouncement { + peer_id: announce_request.peer_id, + peer_addr: std::net::SocketAddr::new(*peer_ip, announce_request.port), + updated: crate::CurrentClock::now(), + uploaded: NumberOfBytes::new(uploaded.0), + downloaded: NumberOfBytes::new(downloaded.0), + left: NumberOfBytes::new(left.0), + event: match &announce_request.event { + Some(event) => match event { + ProtocolAnnounceEvent::Started => AnnounceEvent::Started, + ProtocolAnnounceEvent::Stopped => AnnounceEvent::Stopped, + ProtocolAnnounceEvent::Completed => AnnounceEvent::Completed, + ProtocolAnnounceEvent::Empty => AnnounceEvent::None, + }, + None => AnnounceEvent::None, + }, + } + } + async fn authenticate(&self, maybe_key: Option) -> Result<(), authentication::key::Error> { if self.core_config.private { let key = maybe_key.ok_or(authentication::key::Error::MissingAuthKey { @@ -298,10 +329,25 @@ mod tests { info_hash: sample_info_hash(), peer_id: peer.peer_id, port: peer.peer_addr.port(), - uploaded: Some(peer.uploaded), - downloaded: Some(peer.downloaded), - left: Some(peer.left), - event: Some(peer.event.into()), + uploaded: Some(torrust_tracker_http_tracker_protocol::v1::requests::announce::NumberOfBytes::new(peer.uploaded.0)), + downloaded: Some( + torrust_tracker_http_tracker_protocol::v1::requests::announce::NumberOfBytes::new(peer.downloaded.0), + ), + left: Some(torrust_tracker_http_tracker_protocol::v1::requests::announce::NumberOfBytes::new(peer.left.0)), + event: Some(match peer.event { + torrust_tracker_primitives::AnnounceEvent::Started => { + torrust_tracker_http_tracker_protocol::v1::requests::announce::Event::Started + } + torrust_tracker_primitives::AnnounceEvent::Stopped => { + torrust_tracker_http_tracker_protocol::v1::requests::announce::Event::Stopped + } + torrust_tracker_primitives::AnnounceEvent::Completed => { + torrust_tracker_http_tracker_protocol::v1::requests::announce::Event::Completed + } + torrust_tracker_primitives::AnnounceEvent::None => { + torrust_tracker_http_tracker_protocol::v1::requests::announce::Event::Empty + } + }), compact: None, numwant: None, }; diff --git a/packages/udp-protocol/src/common.rs b/packages/udp-protocol/src/common.rs index 08ccc2493..27a26669d 100644 --- a/packages/udp-protocol/src/common.rs +++ b/packages/udp-protocol/src/common.rs @@ -45,7 +45,9 @@ impl TransactionId { #[derive(PartialEq, Eq, Hash, Clone, Copy, Debug, IntoBytes, FromBytes, Immutable)] #[repr(transparent)] // Intentionally kept in `common`: this mirrors -// `packages/primitives/src/number_of_bytes.rs` and may be shared across packages later. +// `packages/primitives/src/number_of_bytes.rs` and HTTP protocol byte counters, +// but remains UDP-local so protocol wire representations can evolve +// independently per protocol. pub struct NumberOfBytes(pub I64); impl NumberOfBytes { From 4387109cac926014045fffd40c3dba88edd720a1 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 27 May 2026 19:41:47 +0100 Subject: [PATCH 2/2] chore(http): address Copilot PR review comments --- packages/http-protocol/src/v1/requests/announce.rs | 4 ++-- packages/http-tracker-core/src/services/announce.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/http-protocol/src/v1/requests/announce.rs b/packages/http-protocol/src/v1/requests/announce.rs index b895e027c..6400e264f 100644 --- a/packages/http-protocol/src/v1/requests/announce.rs +++ b/packages/http-protocol/src/v1/requests/announce.rs @@ -39,8 +39,8 @@ impl NumberOfBytes { } } -/// The `Announce` request. Fields use the domain types after parsing the -/// query params of the request. +/// The `Announce` request. Fields use protocol-local types after parsing the +/// query params of the request; boundary layers map them to domain types. /// /// ```rust /// use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event}; diff --git a/packages/http-tracker-core/src/services/announce.rs b/packages/http-tracker-core/src/services/announce.rs index 476de159c..df6634039 100644 --- a/packages/http-tracker-core/src/services/announce.rs +++ b/packages/http-tracker-core/src/services/announce.rs @@ -11,7 +11,6 @@ use std::panic::Location; use std::sync::Arc; use bittorrent_primitives::info_hash::InfoHash; -use torrust_clock::clock::Time; use torrust_net_primitives::service_binding::ServiceBinding; use torrust_tracker_configuration::Core; use torrust_tracker_core::announce_handler::{AnnounceHandler, PeersWanted}; @@ -123,7 +122,7 @@ impl AnnounceService { PeerAnnouncement { peer_id: announce_request.peer_id, peer_addr: std::net::SocketAddr::new(*peer_ip, announce_request.port), - updated: crate::CurrentClock::now(), + updated: ::now(), uploaded: NumberOfBytes::new(uploaded.0), downloaded: NumberOfBytes::new(downloaded.0), left: NumberOfBytes::new(left.0),