From 54f83563012fa6432ec4759da857b7fdc183a213 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 27 May 2026 08:58:39 +0100 Subject: [PATCH 1/2] refactor(http-protocol): decouple from tracker-core --- Cargo.lock | 1 - .../open/1669-overhaul-packages/EPIC.md | 55 +++++++++---------- ...ecouple-http-protocol-from-tracker-core.md | 52 +++++++++--------- .../src/v1/handlers/announce.rs | 33 +++-------- .../src/v1/handlers/scrape.rs | 12 +--- .../axum-http-server/tests/server/asserts.rs | 2 +- packages/http-protocol/Cargo.toml | 1 - .../http-protocol/src/v1/responses/error.rs | 32 ----------- .../src/services/announce.rs | 23 ++++++++ .../http-tracker-core/src/services/scrape.rs | 23 ++++++++ 10 files changed, 112 insertions(+), 122 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dfcb56725..388daaf02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5341,7 +5341,6 @@ dependencies = [ "torrust-clock", "torrust-located-error", "torrust-tracker-contrib-bencode", - "torrust-tracker-core", "torrust-tracker-primitives", "torrust-tracker-udp-tracker-protocol", ] diff --git a/docs/issues/open/1669-overhaul-packages/EPIC.md b/docs/issues/open/1669-overhaul-packages/EPIC.md index 0ad2476f1..c663155de 100644 --- a/docs/issues/open/1669-overhaul-packages/EPIC.md +++ b/docs/issues/open/1669-overhaul-packages/EPIC.md @@ -6,7 +6,7 @@ priority: p1 github-issue: 1669 spec-path: docs/issues/open/1669-overhaul-packages/EPIC.md epic-owner: josecelano -last-updated-utc: 2026-05-26 20:15 +last-updated-utc: 2026-05-27 00:00 semantic-links: skill-links: - create-issue @@ -239,9 +239,9 @@ The following crates remain in `torrust/torrust-tracker` for now: - `torrust-tracker-core` Rationale: current dependencies indicate unresolved layering/coupling. In particular, -`torrust-http-tracker-protocol` currently depends on `torrust-tracker-core`, -`torrust-tracker-primitives`, and `torrust-udp-tracker-protocol`. The move can be revisited -after these dependencies are clarified and reduced. +`torrust-http-tracker-protocol` currently depends on +`torrust-tracker-primitives` and `torrust-udp-tracker-protocol`. The move can be +revisited after these dependencies are clarified and reduced. > **Naming policy**: prefix reflects ownership and release identity, not estimated > reusability. Tracker-owned packages keep the `torrust-tracker-` prefix even when they @@ -351,7 +351,6 @@ This section lists direct crate dependencies that have a `torrust*` prefix. - `torrust-bencode` - `torrust-clock` - `torrust-located-error` - - `torrust-tracker-core` - `torrust-tracker-primitives` - `torrust-tracker-udp-tracker-protocol` - `torrust-tracker-udp-tracker-core` @@ -510,7 +509,7 @@ Every subissue touching package boundaries should include: Current known smell to prioritize under these rules: -- `http-protocol` depending on `tracker-core` and `udp-protocol`. +- `http-protocol` depending on `udp-protocol`. ### Quick list @@ -531,7 +530,7 @@ Status: TODO unless noted. #### 2. Open GitHub Issue - [x] [#1829](https://github.com/torrust/torrust-tracker/issues/1829) SI-11: Rename crates and folder names to match desired `torrust-tracker` workspace state _(Rule U; one package at a time)_ -- [ ] [#1830](https://github.com/torrust/torrust-tracker/issues/1830) SI-12: Decouple `http-protocol` from `tracker-core` _(Rule M; remove forbidden `protocol -> tracker-core` edge)_ +- [x] [#1830](https://github.com/torrust/torrust-tracker/issues/1830) SI-12: Decouple `http-protocol` from `tracker-core` _(Rule M; remove forbidden `protocol -> tracker-core` edge)_ #### 3. Numbered Subissue (No GitHub Issue Yet) @@ -549,27 +548,27 @@ Status: TODO unless noted. Details: -| Item | Issue | Local Spec | Status | Notes | -| -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------- | -| Baseline analysis | #TBD — Establish baseline: dependency graph + README audit | [docs/issues/drafts/1669-01-establish-baseline-analysis.md](../../drafts/1669-01-establish-baseline-analysis.md) | TODO | No blockers; informs extraction decisions | -| Duration move | [#1790](https://github.com/torrust/torrust-tracker/issues/1790) — Move `DurationSinceUnixEpoch` from `torrust-tracker-primitives` to `torrust-tracker-clock` | [docs/issues/open/1790-move-duration-since-unix-epoch-to-torrust-tracker-clock.md](../../open/1790-move-duration-since-unix-epoch-to-torrust-tracker-clock.md) | DONE | Rule M; no hard blockers; prerequisite for clock extraction | -| Timeout constants | [#1793](https://github.com/torrust/torrust-tracker/issues/1793) — Define per-package default timeout constants and remove `DEFAULT_TIMEOUT` from `torrust-tracker-configuration` | [docs/issues/open/1793-1669-03-define-per-package-default-timeout-constants.md](../../open/1793-1669-03-define-per-package-default-timeout-constants.md) | DONE | Rule M; completed | -| Announce policy move | [#1795](https://github.com/torrust/torrust-tracker/issues/1795) — Move `AnnouncePolicy` from `torrust-tracker-configuration` to `torrust-tracker-primitives` | [docs/issues/open/1795-1669-04-move-announce-policy-to-torrust-tracker-primitives.md](../../open/1795-1669-04-move-announce-policy-to-torrust-tracker-primitives.md) | DONE | Rule M; completed | -| Net primitives split | [#1797](https://github.com/torrust/torrust-tracker/issues/1797) — Create `torrust-net-primitives` and move `ServiceBinding` from `torrust-tracker-primitives` | [docs/issues/closed/1797-1669-05-create-torrust-net-primitives-and-move-service-binding.md](../../closed/1797-1669-05-create-torrust-net-primitives-and-move-service-binding.md) | DONE | Rule M + new package; generic networking type; completed | -| Layer violation fix | [#1813](https://github.com/torrust/torrust-tracker/issues/1813) — Resolve `torrust-tracker-core` ↔ `torrust-tracker-rest-api-client` layer violation | [docs/issues/closed/1813-1669-06-resolve-torrust-tracker-core-rest-api-layer-violation.md](../../closed/1813-1669-06-resolve-torrust-tracker-core-rest-api-layer-violation.md) | DONE | Rule M; stale unused dev dep removed in PR #1804; unblocks `torrust-tracker-core` extraction | -| Prefix alignment | [#1816](https://github.com/torrust/torrust-tracker/issues/1816) — Align `torrust-` prefix: rename 7 tracker-specific packages to `torrust-tracker-` | [docs/issues/open/1816-1669-07-align-torrust-prefix-rename-tracker-specific-packages.md](../../open/1816-1669-07-align-torrust-prefix-rename-tracker-specific-packages.md) | DONE | Rule U; none of the 7 are published; pure workspace rename; no blockers | -| Metrics rename | [#1819](https://github.com/torrust/torrust-tracker/issues/1819) — Rename `torrust-tracker-metrics` to `torrust-metrics` | [docs/issues/open/1819-1669-08-rename-torrust-tracker-metrics-to-torrust-metrics.md](../../open/1819-1669-08-rename-torrust-tracker-metrics-to-torrust-metrics.md) | DONE | Rule U; not yet published; no blockers; prerequisite for metrics extraction | -| Clock rename | [#1821](https://github.com/torrust/torrust-tracker/issues/1821) — Rename `torrust-tracker-clock` to `torrust-clock` | [docs/issues/open/1821-1669-09-rename-torrust-tracker-clock-to-torrust-clock.md](../../open/1821-1669-09-rename-torrust-tracker-clock-to-torrust-clock.md) | DONE | Rule P; published on crates.io; no blockers; prerequisite for clock extraction | -| Located error rename | [#1823](https://github.com/torrust/torrust-tracker/issues/1823) — Rename `torrust-tracker-located-error` to `torrust-located-error` | [docs/issues/closed/1823-1669-10-rename-torrust-tracker-located-error-to-torrust-located-error.md](../../closed/1823-1669-10-rename-torrust-tracker-located-error-to-torrust-located-error.md) | DONE | Rule P; completed | -| README refresh | #TBD — Update all package READMEs | [docs/issues/drafts/1669-update-all-package-readmes.md](../../drafts/1669-update-all-package-readmes.md) | TODO | Documentation; requires completed rename work; before extraction work | -| Bencode migration | #TBD — Migrate `contrib/bencode` to `torrust/torrust-bittorrent` and replace legacy `packages/bencode` | [docs/issues/drafts/1669-extract-torrust-tracker-contrib-bencode-to-torrust-bencode.md](../../drafts/1669-extract-torrust-tracker-contrib-bencode-to-torrust-bencode.md) | TODO | Rule E; replaces old `torrust-bittorrent` implementation with newer tracker lineage | -| Clock extraction | #TBD — Extract `torrust-clock` to standalone repository | [docs/issues/drafts/1669-extract-torrust-clock-to-standalone-repo.md](../../drafts/1669-extract-torrust-clock-to-standalone-repo.md) | TODO | Rule E; requires completed duration move and clock rename; 11 workspace consumers to migrate | -| Metrics extraction | #TBD — Extract `torrust-metrics` to standalone repository | [docs/issues/drafts/1669-extract-torrust-metrics-to-standalone-repo.md](../../drafts/1669-extract-torrust-metrics-to-standalone-repo.md) | TODO | Rule E; requires completed metrics rename; 7 workspace consumers to migrate | -| Tracker client extraction | #TBD — Extract `torrust-tracker-client` to standalone repository | [docs/issues/drafts/1669-extract-torrust-tracker-client-to-standalone-repo.md](../../drafts/1669-extract-torrust-tracker-client-to-standalone-repo.md) | TODO | Rule E; blocked by `torrust-tracker-udp-tracker-protocol` publication (external to this EPIC) | -| 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/open/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md](../../open/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md) | TODO | SI-11. Rule U; crate-only or folder-only rename per package; execute one package at a time | -| HTTP protocol decoupling | [#1830](https://github.com/torrust/torrust-tracker/issues/1830) — Decouple `http-protocol` from `tracker-core` | [docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md](../../open/1830-1669-12-decouple-http-protocol-from-tracker-core.md) | TODO | SI-12. Rule M; remove forbidden `protocol -> tracker-core` dependency edge | -| HTTP/UDP decoupling | #TBD — Decouple `http-protocol` from `udp-protocol` | [docs/issues/drafts/1669-13-decouple-http-protocol-from-udp-protocol.md](../../drafts/1669-13-decouple-http-protocol-from-udp-protocol.md) | TODO | SI-13. Rule M; remove cross-protocol dependency edge | -| HTTP/primitives decoupling | #TBD — Decouple `http-protocol` from `torrust-tracker-primitives` | [docs/issues/drafts/1669-14-decouple-http-protocol-from-tracker-primitives.md](../../drafts/1669-14-decouple-http-protocol-from-tracker-primitives.md) | TODO | SI-14. Rule M; remove protocol -> domain coupling in step 2 | +| Item | Issue | Local Spec | Status | Notes | +| -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ---------------------------------------------------------------------------------------------- | +| Baseline analysis | #TBD — Establish baseline: dependency graph + README audit | [docs/issues/drafts/1669-01-establish-baseline-analysis.md](../../drafts/1669-01-establish-baseline-analysis.md) | TODO | No blockers; informs extraction decisions | +| Duration move | [#1790](https://github.com/torrust/torrust-tracker/issues/1790) — Move `DurationSinceUnixEpoch` from `torrust-tracker-primitives` to `torrust-tracker-clock` | [docs/issues/open/1790-move-duration-since-unix-epoch-to-torrust-tracker-clock.md](../../open/1790-move-duration-since-unix-epoch-to-torrust-tracker-clock.md) | DONE | Rule M; no hard blockers; prerequisite for clock extraction | +| Timeout constants | [#1793](https://github.com/torrust/torrust-tracker/issues/1793) — Define per-package default timeout constants and remove `DEFAULT_TIMEOUT` from `torrust-tracker-configuration` | [docs/issues/open/1793-1669-03-define-per-package-default-timeout-constants.md](../../open/1793-1669-03-define-per-package-default-timeout-constants.md) | DONE | Rule M; completed | +| Announce policy move | [#1795](https://github.com/torrust/torrust-tracker/issues/1795) — Move `AnnouncePolicy` from `torrust-tracker-configuration` to `torrust-tracker-primitives` | [docs/issues/open/1795-1669-04-move-announce-policy-to-torrust-tracker-primitives.md](../../open/1795-1669-04-move-announce-policy-to-torrust-tracker-primitives.md) | DONE | Rule M; completed | +| Net primitives split | [#1797](https://github.com/torrust/torrust-tracker/issues/1797) — Create `torrust-net-primitives` and move `ServiceBinding` from `torrust-tracker-primitives` | [docs/issues/closed/1797-1669-05-create-torrust-net-primitives-and-move-service-binding.md](../../closed/1797-1669-05-create-torrust-net-primitives-and-move-service-binding.md) | DONE | Rule M + new package; generic networking type; completed | +| Layer violation fix | [#1813](https://github.com/torrust/torrust-tracker/issues/1813) — Resolve `torrust-tracker-core` ↔ `torrust-tracker-rest-api-client` layer violation | [docs/issues/closed/1813-1669-06-resolve-torrust-tracker-core-rest-api-layer-violation.md](../../closed/1813-1669-06-resolve-torrust-tracker-core-rest-api-layer-violation.md) | DONE | Rule M; stale unused dev dep removed in PR #1804; unblocks `torrust-tracker-core` extraction | +| Prefix alignment | [#1816](https://github.com/torrust/torrust-tracker/issues/1816) — Align `torrust-` prefix: rename 7 tracker-specific packages to `torrust-tracker-` | [docs/issues/open/1816-1669-07-align-torrust-prefix-rename-tracker-specific-packages.md](../../open/1816-1669-07-align-torrust-prefix-rename-tracker-specific-packages.md) | DONE | Rule U; none of the 7 are published; pure workspace rename; no blockers | +| Metrics rename | [#1819](https://github.com/torrust/torrust-tracker/issues/1819) — Rename `torrust-tracker-metrics` to `torrust-metrics` | [docs/issues/open/1819-1669-08-rename-torrust-tracker-metrics-to-torrust-metrics.md](../../open/1819-1669-08-rename-torrust-tracker-metrics-to-torrust-metrics.md) | DONE | Rule U; not yet published; no blockers; prerequisite for metrics extraction | +| Clock rename | [#1821](https://github.com/torrust/torrust-tracker/issues/1821) — Rename `torrust-tracker-clock` to `torrust-clock` | [docs/issues/open/1821-1669-09-rename-torrust-tracker-clock-to-torrust-clock.md](../../open/1821-1669-09-rename-torrust-tracker-clock-to-torrust-clock.md) | DONE | Rule P; published on crates.io; no blockers; prerequisite for clock extraction | +| Located error rename | [#1823](https://github.com/torrust/torrust-tracker/issues/1823) — Rename `torrust-tracker-located-error` to `torrust-located-error` | [docs/issues/closed/1823-1669-10-rename-torrust-tracker-located-error-to-torrust-located-error.md](../../closed/1823-1669-10-rename-torrust-tracker-located-error-to-torrust-located-error.md) | DONE | Rule P; completed | +| README refresh | #TBD — Update all package READMEs | [docs/issues/drafts/1669-update-all-package-readmes.md](../../drafts/1669-update-all-package-readmes.md) | TODO | Documentation; requires completed rename work; before extraction work | +| Bencode migration | #TBD — Migrate `contrib/bencode` to `torrust/torrust-bittorrent` and replace legacy `packages/bencode` | [docs/issues/drafts/1669-extract-torrust-tracker-contrib-bencode-to-torrust-bencode.md](../../drafts/1669-extract-torrust-tracker-contrib-bencode-to-torrust-bencode.md) | TODO | Rule E; replaces old `torrust-bittorrent` implementation with newer tracker lineage | +| Clock extraction | #TBD — Extract `torrust-clock` to standalone repository | [docs/issues/drafts/1669-extract-torrust-clock-to-standalone-repo.md](../../drafts/1669-extract-torrust-clock-to-standalone-repo.md) | TODO | Rule E; requires completed duration move and clock rename; 11 workspace consumers to migrate | +| Metrics extraction | #TBD — Extract `torrust-metrics` to standalone repository | [docs/issues/drafts/1669-extract-torrust-metrics-to-standalone-repo.md](../../drafts/1669-extract-torrust-metrics-to-standalone-repo.md) | TODO | Rule E; requires completed metrics rename; 7 workspace consumers to migrate | +| Tracker client extraction | #TBD — Extract `torrust-tracker-client` to standalone repository | [docs/issues/drafts/1669-extract-torrust-tracker-client-to-standalone-repo.md](../../drafts/1669-extract-torrust-tracker-client-to-standalone-repo.md) | TODO | Rule E; blocked by `torrust-tracker-udp-tracker-protocol` publication (external to this EPIC) | +| 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/open/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md](../../open/1829-1669-11-rename-crates-and-folders-to-match-desired-tracker-workspace.md) | TODO | SI-11. Rule U; crate-only or folder-only rename per package; execute one package at a time | +| HTTP protocol decoupling | [#1830](https://github.com/torrust/torrust-tracker/issues/1830) — Decouple `http-protocol` from `tracker-core` | [docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md](../../open/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 | #TBD — Decouple `http-protocol` from `udp-protocol` | [docs/issues/drafts/1669-13-decouple-http-protocol-from-udp-protocol.md](../../drafts/1669-13-decouple-http-protocol-from-udp-protocol.md) | TODO | SI-13. Rule M; remove cross-protocol dependency edge | +| HTTP/primitives decoupling | #TBD — Decouple `http-protocol` from `torrust-tracker-primitives` | [docs/issues/drafts/1669-14-decouple-http-protocol-from-tracker-primitives.md](../../drafts/1669-14-decouple-http-protocol-from-tracker-primitives.md) | TODO | SI-14. Rule M; remove protocol -> domain coupling in step 2 | ### Draft issues diff --git a/docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md b/docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md index ad4311502..e3d8f8aa8 100644 --- a/docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md +++ b/docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md @@ -5,9 +5,9 @@ status: open priority: p1 github-issue: 1830 spec-path: docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md -branch: null +branch: 1830-1669-12-decouple-http-protocol-from-tracker-core related-pr: null -last-updated-utc: 2026-05-26 00:00 +last-updated-utc: 2026-05-27 00:00 semantic-links: skill-links: - create-issue @@ -104,35 +104,35 @@ Usage purpose: Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | -| T1 | TODO | Confirm all tracker-core usage in `http-protocol` is limited to `responses/error.rs` | Evidence captured in PR description | -| T2 | TODO | Remove `From` impls from `packages/http-protocol/src/v1/responses/error.rs` | No direct imports or type references to `bittorrent_tracker_core::*` remain | -| T3 | TODO | Remove `bittorrent-tracker-core` from `packages/http-protocol/Cargo.toml` | `cargo metadata` shows no `http-protocol -> tracker-core` edge | -| T4 | TODO | Add/adjust mapping at higher layer (`http-tracker-core` and/or `axum-http-tracker-server`) for equivalent client-visible failure messages | Existing behavior preserved for announce/scrape/auth/whitelist errors | -| T5 | TODO | Update or add tests for failure mapping behavior | Coverage in affected crates; assertions on error message fragments | -| 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 tracker-core usage in `http-protocol` is limited to `responses/error.rs` | Confirmed by `rg` before edits (`torrust_tracker_core::*` only in `responses/error.rs`) | +| T2 | DONE | Remove `From` impls from `packages/http-protocol/src/v1/responses/error.rs` | Removed announce/scrape/whitelist/authentication conversion impls | +| T3 | DONE | Remove `bittorrent-tracker-core` from `packages/http-protocol/Cargo.toml` | Removed dependency; `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` has no tracker-core edge | +| T4 | DONE | Add/adjust mapping at higher layer (`http-tracker-core` and/or `axum-http-tracker-server`) for equivalent client-visible failure messages | Added `From` and `From` into protocol `responses::error::Error` in `http-tracker-core` | +| T5 | DONE | Update or add tests for failure mapping behavior | Updated axum handler unit/integration assertions to use boundary mapping with expected message fragments | +| T6 | DONE | Run verification commands | `cargo build --workspace`, targeted crate tests, `linter all` all passed | +| T7 | DONE | Update EPIC tracking rows and draft list as needed | Updated in EPIC Active Subissues and details table | +| T8 | DONE | Update EPIC after implementation | Updated EPIC dependency narrative and `torrust-tracker-http-tracker-protocol` direct dependency list | ## Acceptance Criteria -- [ ] `packages/http-protocol/Cargo.toml` has no `bittorrent-tracker-core` dependency. -- [ ] `packages/http-protocol` has no source-level references to `bittorrent_tracker_core::`. -- [ ] Client-visible HTTP error responses still include meaningful failure reasons +- [x] `packages/http-protocol/Cargo.toml` has no `bittorrent-tracker-core` dependency. +- [x] `packages/http-protocol` has no source-level references to `bittorrent_tracker_core::`. +- [x] Client-visible HTTP error responses still include meaningful failure reasons for announce/scrape/auth/whitelist failures. -- [ ] `cargo build --workspace` passes. -- [ ] Relevant tests in HTTP protocol/core/server packages pass. -- [ ] `linter all` exits with code `0`. -- [ ] EPIC tracking is updated to include 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 is updated to include this subissue. ## Verification Plan ### Automatic Checks - `cargo build --workspace` -- `cargo test -p bittorrent-http-tracker-protocol` -- `cargo test -p bittorrent-http-tracker-core` +- `cargo test -p torrust-tracker-http-tracker-protocol` +- `cargo test -p torrust-tracker-http-tracker-core` - `cargo test -p torrust-tracker-axum-http-server` - `linter all` @@ -140,11 +140,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 forbidden edge remains | `cargo tree -p bittorrent-http-tracker-protocol --depth 1` | No dependency on `bittorrent-tracker-core` | TODO | | -| M2 | No tracker-core symbols in protocol source | `rg "bittorrent_tracker_core::" packages/http-protocol` | No matches | TODO | | -| M3 | Error mapping behavior preserved | Trigger announce/scrape/auth failure cases in existing tests | Error responses still include expected message context | TODO | | +| ID | Scenario | Command / Steps | Expected Result | Status | Evidence | +| --- | ------------------------------------------ | ------------------------------------------------------------------------------- | ------------------------------------------------------ | ------ | ---------------------------------------------------------------------------- | +| M1 | No forbidden edge remains | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` | No dependency on `torrust-tracker-core` | DONE | Tree output shows no tracker-core dependency | +| M2 | No tracker-core symbols in protocol source | `rg "torrust_tracker_core::\|bittorrent_tracker_core::" packages/http-protocol` | No matches | DONE | `rg` returned no output | +| M3 | Error mapping behavior preserved | Trigger announce/scrape/auth failure cases in existing tests | Error responses still include expected message context | DONE | `cargo test -p torrust-tracker-axum-http-server` passed (unit + integration) | ## Risks and Trade-offs diff --git a/packages/axum-http-server/src/v1/handlers/announce.rs b/packages/axum-http-server/src/v1/handlers/announce.rs index 707d78b91..51ead5158 100644 --- a/packages/axum-http-server/src/v1/handlers/announce.rs +++ b/packages/axum-http-server/src/v1/handlers/announce.rs @@ -68,9 +68,7 @@ async fn handle( { Ok(announce_data) => announce_data, Err(error) => { - let error_response = responses::error::Error { - failure_reason: error.to_string(), - }; + let error_response = responses::error::Error::from(error); return (StatusCode::OK, error_response.write()).into_response(); } }; @@ -253,14 +251,9 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); - assert_error_response( - &error_response, - "Tracker core error: Tracker core authentication error: Missing authentication key", - ); + assert_error_response(&error_response, "Tracker authentication error: Missing authentication key"); } #[tokio::test] @@ -284,13 +277,11 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, - "Tracker core error: Tracker core authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ", + "Tracker authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ", ); } } @@ -325,14 +316,12 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, &format!( - "Tracker core error: Tracker core whitelist error: The torrent: {}, is not whitelisted", + "Tracker whitelist error: The torrent: {}, is not whitelisted", announce_request.info_hash ), ); @@ -373,9 +362,7 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, @@ -418,9 +405,7 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, diff --git a/packages/axum-http-server/src/v1/handlers/scrape.rs b/packages/axum-http-server/src/v1/handlers/scrape.rs index 63ad975f7..92cbcb81f 100644 --- a/packages/axum-http-server/src/v1/handlers/scrape.rs +++ b/packages/axum-http-server/src/v1/handlers/scrape.rs @@ -61,9 +61,7 @@ async fn handle( { Ok(scrape_data) => scrape_data, Err(error) => { - let error_response = responses::error::Error { - failure_reason: error.to_string(), - }; + let error_response = responses::error::Error::from(error); return (StatusCode::OK, error_response.write()).into_response(); } }; @@ -332,9 +330,7 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, @@ -379,9 +375,7 @@ mod tests { .await .unwrap_err(); - let error_response = responses::error::Error { - failure_reason: response.to_string(), - }; + let error_response = responses::error::Error::from(response); assert_error_response( &error_response, diff --git a/packages/axum-http-server/tests/server/asserts.rs b/packages/axum-http-server/tests/server/asserts.rs index 7ec9e46d6..30ea8f37a 100644 --- a/packages/axum-http-server/tests/server/asserts.rs +++ b/packages/axum-http-server/tests/server/asserts.rs @@ -150,7 +150,7 @@ pub async fn assert_tracker_core_authentication_error_response(response: Respons assert_bencoded_error( &response.text().await.unwrap(), - "Tracker core error: Tracker core authentication error", + "Tracker authentication error", Location::caller(), ); } diff --git a/packages/http-protocol/Cargo.toml b/packages/http-protocol/Cargo.toml index 15c98b07f..7cb96cda4 100644 --- a/packages/http-protocol/Cargo.toml +++ b/packages/http-protocol/Cargo.toml @@ -17,7 +17,6 @@ version.workspace = true [dependencies] torrust-tracker-udp-tracker-protocol = { version = "3.0.0-develop", path = "../udp-protocol" } bittorrent-primitives = "0.2.0" -torrust-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" } derive_more = { version = "2", features = [ "as_ref", "constructor", "from" ] } multimap = "0" percent-encoding = "2" diff --git a/packages/http-protocol/src/v1/responses/error.rs b/packages/http-protocol/src/v1/responses/error.rs index 2548973b2..20d7c8ac9 100644 --- a/packages/http-protocol/src/v1/responses/error.rs +++ b/packages/http-protocol/src/v1/responses/error.rs @@ -64,38 +64,6 @@ impl From for Error { } } -impl From for Error { - fn from(err: torrust_tracker_core::error::AnnounceError) -> Self { - Error { - failure_reason: format!("Tracker announce error: {err}"), - } - } -} - -impl From for Error { - fn from(err: torrust_tracker_core::error::ScrapeError) -> Self { - Error { - failure_reason: format!("Tracker scrape error: {err}"), - } - } -} - -impl From for Error { - fn from(err: torrust_tracker_core::error::WhitelistError) -> Self { - Error { - failure_reason: format!("Tracker whitelist error: {err}"), - } - } -} - -impl From for Error { - fn from(err: torrust_tracker_core::authentication::Error) -> Self { - Error { - failure_reason: format!("Tracker authentication error: {err}"), - } - } -} - #[cfg(test)] mod tests { use std::panic::Location; diff --git a/packages/http-tracker-core/src/services/announce.rs b/packages/http-tracker-core/src/services/announce.rs index cf8fce42e..1616509ee 100644 --- a/packages/http-tracker-core/src/services/announce.rs +++ b/packages/http-tracker-core/src/services/announce.rs @@ -19,6 +19,7 @@ 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::responses::error::Error as HttpProtocolErrorResponse; use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::{ ClientIpSources, PeerIpResolutionError, RemoteClientAddr, resolve_remote_client_addr, }; @@ -201,6 +202,28 @@ impl From for HttpAnnounceError { } } +impl From for HttpProtocolErrorResponse { + fn from(error: HttpAnnounceError) -> Self { + match error { + HttpAnnounceError::PeerIpResolutionError { source } => source.into(), + HttpAnnounceError::TrackerCoreError { source } => match source { + TrackerCoreError::AnnounceError { source } => Self { + failure_reason: format!("Tracker announce error: {source}"), + }, + TrackerCoreError::ScrapeError { source } => Self { + failure_reason: format!("Tracker scrape error: {source}"), + }, + TrackerCoreError::WhitelistError { source } => Self { + failure_reason: format!("Tracker whitelist error: {source}"), + }, + TrackerCoreError::AuthenticationError { source } => Self { + failure_reason: format!("Tracker authentication error: {source}"), + }, + }, + } + } +} + #[cfg(test)] mod tests { use std::net::SocketAddr; diff --git a/packages/http-tracker-core/src/services/scrape.rs b/packages/http-tracker-core/src/services/scrape.rs index de315bc38..fc46c77b0 100644 --- a/packages/http-tracker-core/src/services/scrape.rs +++ b/packages/http-tracker-core/src/services/scrape.rs @@ -16,6 +16,7 @@ use torrust_tracker_core::authentication::{self, Key}; use torrust_tracker_core::error::{ScrapeError, TrackerCoreError, WhitelistError}; use torrust_tracker_core::scrape_handler::ScrapeHandler; use torrust_tracker_http_tracker_protocol::v1::requests::scrape::Scrape; +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, }; @@ -163,6 +164,28 @@ impl From for HttpScrapeError { } } +impl From for HttpProtocolErrorResponse { + fn from(error: HttpScrapeError) -> Self { + match error { + HttpScrapeError::PeerIpResolutionError { source } => source.into(), + HttpScrapeError::TrackerCoreError { source } => match source { + TrackerCoreError::AnnounceError { source } => Self { + failure_reason: format!("Tracker announce error: {source}"), + }, + TrackerCoreError::ScrapeError { source } => Self { + failure_reason: format!("Tracker scrape error: {source}"), + }, + TrackerCoreError::WhitelistError { source } => Self { + failure_reason: format!("Tracker whitelist error: {source}"), + }, + TrackerCoreError::AuthenticationError { source } => Self { + failure_reason: format!("Tracker authentication error: {source}"), + }, + }, + } + } +} + #[cfg(test)] mod tests { From 19b0ce177d9a51a00206f073f0f24ba32943c2f6 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 27 May 2026 09:33:44 +0100 Subject: [PATCH 2/2] refactor(http-tracker-core): deduplicate protocol error mapping --- .../axum-http-server/tests/server/asserts.rs | 8 +------- .../src/services/announce.rs | 16 ++-------------- .../src/services/error_mapping.rs | 19 +++++++++++++++++++ .../http-tracker-core/src/services/mod.rs | 1 + .../http-tracker-core/src/services/scrape.rs | 16 ++-------------- 5 files changed, 25 insertions(+), 35 deletions(-) create mode 100644 packages/http-tracker-core/src/services/error_mapping.rs diff --git a/packages/axum-http-server/tests/server/asserts.rs b/packages/axum-http-server/tests/server/asserts.rs index 30ea8f37a..44a8494cc 100644 --- a/packages/axum-http-server/tests/server/asserts.rs +++ b/packages/axum-http-server/tests/server/asserts.rs @@ -146,11 +146,5 @@ pub async fn assert_authentication_error_response(response: Response) { } pub async fn assert_tracker_core_authentication_error_response(response: Response) { - assert_eq!(response.status(), 200); - - assert_bencoded_error( - &response.text().await.unwrap(), - "Tracker authentication error", - Location::caller(), - ); + assert_authentication_error_response(response).await; } diff --git a/packages/http-tracker-core/src/services/announce.rs b/packages/http-tracker-core/src/services/announce.rs index 1616509ee..65ccf12bd 100644 --- a/packages/http-tracker-core/src/services/announce.rs +++ b/packages/http-tracker-core/src/services/announce.rs @@ -28,6 +28,7 @@ use torrust_tracker_primitives::peer::PeerAnnouncement; use crate::event; use crate::event::Event; +use crate::services::error_mapping::protocol_error_from_tracker_core_error; /// The HTTP tracker `announce` service. /// @@ -206,20 +207,7 @@ impl From for HttpProtocolErrorResponse { fn from(error: HttpAnnounceError) -> Self { match error { HttpAnnounceError::PeerIpResolutionError { source } => source.into(), - HttpAnnounceError::TrackerCoreError { source } => match source { - TrackerCoreError::AnnounceError { source } => Self { - failure_reason: format!("Tracker announce error: {source}"), - }, - TrackerCoreError::ScrapeError { source } => Self { - failure_reason: format!("Tracker scrape error: {source}"), - }, - TrackerCoreError::WhitelistError { source } => Self { - failure_reason: format!("Tracker whitelist error: {source}"), - }, - TrackerCoreError::AuthenticationError { source } => Self { - failure_reason: format!("Tracker authentication error: {source}"), - }, - }, + HttpAnnounceError::TrackerCoreError { source } => protocol_error_from_tracker_core_error(source), } } } diff --git a/packages/http-tracker-core/src/services/error_mapping.rs b/packages/http-tracker-core/src/services/error_mapping.rs new file mode 100644 index 000000000..3dd7cb473 --- /dev/null +++ b/packages/http-tracker-core/src/services/error_mapping.rs @@ -0,0 +1,19 @@ +use torrust_tracker_core::error::TrackerCoreError; +use torrust_tracker_http_tracker_protocol::v1::responses::error::Error as HttpProtocolErrorResponse; + +pub(crate) fn protocol_error_from_tracker_core_error(error: TrackerCoreError) -> HttpProtocolErrorResponse { + match error { + TrackerCoreError::AnnounceError { source } => HttpProtocolErrorResponse { + failure_reason: format!("Tracker announce error: {source}"), + }, + TrackerCoreError::ScrapeError { source } => HttpProtocolErrorResponse { + failure_reason: format!("Tracker scrape error: {source}"), + }, + TrackerCoreError::WhitelistError { source } => HttpProtocolErrorResponse { + failure_reason: format!("Tracker whitelist error: {source}"), + }, + TrackerCoreError::AuthenticationError { source } => HttpProtocolErrorResponse { + failure_reason: format!("Tracker authentication error: {source}"), + }, + } +} diff --git a/packages/http-tracker-core/src/services/mod.rs b/packages/http-tracker-core/src/services/mod.rs index ce99c6856..8dcab032b 100644 --- a/packages/http-tracker-core/src/services/mod.rs +++ b/packages/http-tracker-core/src/services/mod.rs @@ -6,4 +6,5 @@ //! //! Refer to [`torrust_tracker`](crate) documentation. pub mod announce; +pub(crate) mod error_mapping; pub mod scrape; diff --git a/packages/http-tracker-core/src/services/scrape.rs b/packages/http-tracker-core/src/services/scrape.rs index fc46c77b0..d7454390d 100644 --- a/packages/http-tracker-core/src/services/scrape.rs +++ b/packages/http-tracker-core/src/services/scrape.rs @@ -23,6 +23,7 @@ use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::{ use torrust_tracker_primitives::ScrapeData; use crate::event::{ConnectionContext, Event}; +use crate::services::error_mapping::protocol_error_from_tracker_core_error; /// The HTTP tracker `scrape` service. /// @@ -168,20 +169,7 @@ impl From for HttpProtocolErrorResponse { fn from(error: HttpScrapeError) -> Self { match error { HttpScrapeError::PeerIpResolutionError { source } => source.into(), - HttpScrapeError::TrackerCoreError { source } => match source { - TrackerCoreError::AnnounceError { source } => Self { - failure_reason: format!("Tracker announce error: {source}"), - }, - TrackerCoreError::ScrapeError { source } => Self { - failure_reason: format!("Tracker scrape error: {source}"), - }, - TrackerCoreError::WhitelistError { source } => Self { - failure_reason: format!("Tracker whitelist error: {source}"), - }, - TrackerCoreError::AuthenticationError { source } => Self { - failure_reason: format!("Tracker authentication error: {source}"), - }, - }, + HttpScrapeError::TrackerCoreError { source } => protocol_error_from_tracker_core_error(source), } } }