Skip to content

Commit 3e5b381

Browse files
committed
docs(issues): refine epic subissue specifications
1 parent f8fa9bf commit 3e5b381

7 files changed

Lines changed: 95 additions & 32 deletions

docs/issues/1532-http-tracker-client-add-optional-announce-params.md

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,28 +61,39 @@ cargo run -p torrust-tracker-client --bin http_tracker_client announce \
6161

6262
Supported `--event` values: `started`, `stopped`, `completed` (case-insensitive).
6363

64+
`--peer-id` input contract:
65+
66+
- Accept a 20-character ASCII value.
67+
- Reject any value that is not exactly 20 bytes.
68+
- Surface validation errors as CLI argument errors.
69+
6470
## Goals
6571

6672
- [ ] Add optional CLI flags to the `announce` sub-command in
6773
`console/tracker-client/src/console/clients/http/app.rs`:
6874
`--event`, `--uploaded`, `--downloaded`, `--left`, `--port`, `--peer-addr`,
6975
`--peer-id`, `--compact`
70-
- [ ] Parse each flag and pass its value to the corresponding `QueryBuilder` setter
76+
- [ ] Parse each flag and map it into `announce::Query` values
77+
- [ ] Extend `QueryBuilder` with missing setters for
78+
`event`, `uploaded`, `downloaded`, `left`, and `port`
7179
- [ ] Defaults remain unchanged when a flag is omitted
72-
- [ ] Add `FromStr` / `clap` value parsing for `Event` (already has `Display`; needs `FromStr`)
80+
- [ ] Add CLI parsing for `Event` in the tracker-client layer
7381
- [ ] Pass `linter all` and `cargo machete` with zero warnings
7482
- [ ] Update the module-level doc comment in `app.rs` with new usage examples
7583

7684
## Implementation Plan
7785

78-
### Task 1: Add `FromStr` for `Event`
86+
### Task 1: Add CLI parsing for `Event`
7987

80-
`Event` already implements `Display`. Add a `FromStr` implementation (or derive it via `clap`'s
81-
`ValueEnum`) so it can be parsed directly from the command line.
88+
Use a CLI-facing enum (for example `CliEvent`) in
89+
`console/tracker-client/src/console/clients/http/app.rs` and map it into
90+
`bittorrent_tracker_client::http::client::requests::announce::Event`.
8291

83-
- [ ] Implement `clap::ValueEnum` for `Event` in
84-
`packages/tracker-client/src/http/client/requests/announce.rs`
85-
(or add `FromStr` and map it in the CLI layer)
92+
Do not rely on `packages/http-protocol` `Event`, which is a different type and
93+
belongs to a different layer.
94+
95+
- [ ] Implement `clap::ValueEnum` for the CLI-facing `event` type
96+
- [ ] Add explicit mapping from CLI event type to tracker-client request `Event`
8697

8798
### Task 2: Extend the `Announce` sub-command struct
8899

@@ -95,7 +106,7 @@ Announce {
95106
tracker_url: String,
96107
info_hash: String,
97108
#[arg(long)]
98-
event: Option<Event>,
109+
event: Option<CliEvent>,
99110
#[arg(long)]
100111
uploaded: Option<u64>,
101112
#[arg(long)]
@@ -109,14 +120,20 @@ Announce {
109120
#[arg(long, name = "peer-id")]
110121
peer_id: Option<String>,
111122
#[arg(long)]
112-
compact: Option<u8>,
123+
compact: Option<CliCompact>,
113124
}
114125
```
115126

127+
`CliCompact` should accept only `0` and `1` and map to
128+
`announce::Compact::{NotAccepted, Accepted}`.
129+
116130
### Task 3: Thread optional values through `announce_command`
117131

118132
- [ ] Update `announce_command` signature to accept the optional parameters
133+
- [ ] Add missing `QueryBuilder` setters in
134+
`packages/tracker-client/src/http/client/requests/announce.rs`
119135
- [ ] Apply each `Some(value)` to the `QueryBuilder` chain before calling `.query()`
136+
- [ ] Parse and validate `--peer-id` into `bittorrent_udp_tracker_protocol::PeerId`
120137

121138
### Task 4: Update docs
122139

docs/issues/1533-udp-tracker-client-add-optional-announce-params.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ cargo run -p torrust-tracker-client --bin udp_tracker_client announce \
6464
Supported `--event` values: `none`, `completed`, `started`, `stopped` (matching
6565
`bittorrent_udp_tracker_protocol::AnnounceEvent` variants, case-insensitive).
6666

67+
`--peer-id` input contract:
68+
69+
- Accept a 20-character ASCII value.
70+
- Reject any value that is not exactly 20 bytes.
71+
- Surface validation errors as CLI argument errors.
72+
6773
## Goals
6874

6975
- [ ] Add optional CLI flags to the `Announce` variant in
@@ -137,6 +143,8 @@ Announce {
137143
`checker::Client::send_announce_request()`
138144
- [ ] Update `send_announce_request` in `checker.rs` to accept an optional parameter struct
139145
(or individual `Option` arguments) and apply overrides when `Some`
146+
- [ ] Validate and parse `--peer-id` into `bittorrent_udp_tracker_protocol::PeerId`
147+
- [ ] Reject negative values for `uploaded`, `downloaded`, and `left` at the CLI layer
140148

141149
### Task 4: Update docs
142150

docs/issues/1561-http-tracker-client-avoid-duplicating-announce-suffix.md

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ accept both forms:
1313
- base URL, for example `https://tracker.torrust-demo.com/`
1414
- full announce URL, for example `https://tracker.torrust-demo.com/announce`
1515

16+
The `/announce` suffix is common in public tracker lists (for example
17+
newtrackon), but not guaranteed by protocol-level requirements. The client
18+
should therefore support a mixed strategy:
19+
20+
- If the input URL path is empty (domain only) or exactly `/`, append
21+
`/announce`.
22+
- If the input URL already contains a path segment, keep it as provided.
23+
1624
- GitHub issue: <https://github.com/torrust/torrust-tracker/issues/1561>
1725
- Parent EPIC: <https://github.com/torrust/torrust-tracker/issues/669>
1826

@@ -69,33 +77,35 @@ Expected accepted inputs for announce:
6977
- `https://tracker.torrust-demo.com`
7078
- `https://tracker.torrust-demo.com/`
7179
- `https://tracker.torrust-demo.com/announce`
80+
- `https://tracker.torrust-demo.com/custom-tracker-endpoint`
7281

7382
Expected final request path for announce:
7483

75-
- exactly one `/announce`
76-
77-
Expected accepted inputs for scrape:
78-
79-
- `https://tracker.torrust-demo.com`
80-
- `https://tracker.torrust-demo.com/`
81-
- `https://tracker.torrust-demo.com/scrape`
84+
- exactly one effective endpoint path, resolved by the rule below
8285

83-
Expected final request path for scrape:
86+
Path resolution rule for `announce`:
8487

85-
- exactly one `/scrape`
88+
- Input path empty or `/` -> resolve to `/announce`
89+
- Input path non-empty (for example `/announce`, `/foo`, `/foo/bar`) -> keep it
90+
unchanged
8691

8792
The client should not rely on callers pre-trimming or pre-normalizing the URL.
8893

94+
Scope note: this issue is about tracker protocol endpoints (`announce` and
95+
`scrape`). The `health_check` endpoint is out of scope.
96+
8997
## Goals
9098

9199
- [ ] Accept both bare tracker base URLs and full announce URLs in the HTTP
92100
client
101+
- [ ] Append `/announce` only for bare URLs (`host` or `host/`)
102+
- [ ] Keep provided path unchanged when a non-empty path already exists
93103
- [ ] Avoid duplicating the `announce` path suffix in the final request URL
94-
- [ ] Avoid duplicating the `scrape` path suffix in the final request URL
95104
- [ ] Keep authenticated path handling working, including URLs that append the
96105
authentication key after the endpoint path
97106
- [ ] Preserve existing behaviour for valid base URLs
98107
- [ ] Add tests covering the supported input forms
108+
- [ ] Keep `health_check` behaviour unchanged in this issue
99109
- [ ] `linter all` exits with code `0`
100110
- [ ] `cargo machete` reports no unused dependencies
101111
- [ ] Existing tests pass
@@ -117,12 +127,14 @@ Instead, add a helper that derives a normalized endpoint URL from the parsed
117127

118128
The key rule is: the final URL must contain the endpoint suffix exactly once.
119129

120-
### Task 2: Normalize announce and scrape independently
130+
### Task 2: Apply base-URL detection for announce
131+
132+
For announce requests:
121133

122-
Ensure announce requests always resolve to exactly one `announce` segment and
123-
scrape requests always resolve to exactly one `scrape` segment.
134+
- If the input URL path is empty or `/`, append `announce`
135+
- Otherwise, keep the original path unchanged
124136

125-
Do not implement a narrow fix that only handles `announce`.
137+
Do not append `announce` when any path segment already exists.
126138

127139
### Task 3: Preserve authenticated endpoint support
128140

@@ -151,8 +163,7 @@ Add tests in `packages/tracker-client/src/http/client/mod.rs` covering at least:
151163
- base URL without trailing slash + announce
152164
- base URL with trailing slash + announce
153165
- full `/announce` URL + announce
154-
- base URL without trailing slash + scrape
155-
- full `/scrape` URL + scrape
166+
- full custom path URL + announce (path unchanged)
156167
- authenticated announce path with a full `/announce` base URL
157168

158169
The tests should assert the exact final URL string.
@@ -163,16 +174,20 @@ Update the module docs in
163174
`console/tracker-client/src/console/clients/http/app.rs` or package docs so the
164175
accepted URL forms are explicit.
165176

177+
### Task 6: Keep `health_check` out of scope
178+
179+
Do not change `health_check` behavior as part of this bug fix. If endpoint
180+
normalization is later generalized to all methods, that should be handled in a
181+
separate issue with dedicated tests.
182+
166183
## Acceptance Criteria
167184

168185
- [ ] Passing `https://tracker.torrust-demo.com` to the announce command sends
169186
the request to `/announce`
170187
- [ ] Passing `https://tracker.torrust-demo.com/announce` to the announce
171188
command also sends the request to `/announce`
172-
- [ ] Passing `https://tracker.torrust-demo.com` to the scrape command sends
173-
the request to `/scrape`
174-
- [ ] Passing `https://tracker.torrust-demo.com/scrape` to the scrape command
175-
also sends the request to `/scrape`
189+
- [ ] Passing a URL with a non-empty path (for example `/foo`) keeps `/foo`
190+
unchanged and does not append `announce`
176191
- [ ] Authenticated requests still generate correct URLs
177192
- [ ] No duplicated endpoint suffix appears in final request URLs
178193
- [ ] `linter all` exits with code `0`

docs/issues/1562-http-tracker-client-add-option-show-response-pretty-json.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ Add `--format` to HTTP commands with the following values:
4141
- `compact` (default)
4242
- `pretty`
4343

44+
Formatting applies to both typed responses and fallback JSON generated for
45+
unrecognized responses (from #672). Raw-byte fallback remains plain text and is
46+
not reformatted.
47+
4448
Defaulting to `compact` is intentional because:
4549

4650
- It is better for shell pipelines and machine parsing.
@@ -121,6 +125,8 @@ Update examples in `app.rs` module docs to include `--format pretty` usage.
121125
- [ ] `announce --format pretty` prints multiline indented JSON
122126
- [ ] `scrape --format pretty` prints multiline indented JSON
123127
- [ ] Omitting `--format` still produces compact single-line JSON
128+
- [ ] When fallback JSON is produced, `--format pretty` prints indented JSON and
129+
default output remains compact
124130
- [ ] Invalid format values are rejected by clap with usage guidance
125131
- [ ] `linter all` exits with code `0`
126132
- [ ] `cargo machete` reports no unused dependencies

docs/issues/1563-udp-tracker-client-add-option-show-response-pretty-json.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ Add `--format` to UDP commands with values:
4343
- `compact` (default)
4444
- `pretty`
4545

46+
Formatting applies to both typed responses and fallback JSON generated for
47+
unrecognized responses (from #671 style behavior). Raw-byte fallback remains
48+
plain text and is not reformatted.
49+
4650
Defaulting to `compact` is intentional because:
4751

4852
- It is better for shell pipelines and machine parsing.
@@ -128,6 +132,8 @@ Update examples to show both default and explicit `--format pretty` usage.
128132
- [ ] Running UDP `announce --format compact` prints single-line JSON
129133
- [ ] Running UDP `scrape --format pretty` prints multiline JSON
130134
- [ ] Omitting `--format` produces compact single-line JSON
135+
- [ ] When fallback JSON is produced, `--format pretty` prints indented JSON and
136+
default output remains compact
131137
- [ ] Invalid format values are rejected by clap with usage guidance
132138
- [ ] `linter all` exits with code `0`
133139
- [ ] `cargo machete` reports no unused dependencies

docs/issues/672-http-tracker-client-print-unrecognized-responses.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ let scrape_response = scrape::Response::try_from_bencoded(&body)
4747
`scrape::Response::try_from_bencoded` also panics internally via
4848
`serde_bencode::from_bytes(bytes).expect(...)`.
4949

50+
The scrape parser path also contains nested `.unwrap()` calls while iterating
51+
decoded file dictionaries. Those must be removed from reachable runtime paths.
52+
5053
## Proposed Behaviour
5154

5255
The two-step fallback strategy:
@@ -80,14 +83,17 @@ Warning: Could not deserialize HTTP tracker response. Raw bytes: [100, 56, ...]
8083

8184
- [ ] Replace both `panic!(...)` / `.unwrap_or_else(|_| panic!(...))` calls in `app.rs` with
8285
graceful fallback logic
83-
- [ ] Remove the `panic!` inside `scrape::Response::try_from_bencoded`; change the internal
84-
`expect(...)` to return `Err` properly
86+
- [ ] Remove panic/unwrap usage from the scrape decode path:
87+
`expect(...)` in `try_from_bencoded` and nested `.unwrap()` calls in
88+
parser helpers
8589
- [ ] Add `bencode2json` as a dependency of the `torrust-tracker-client` console crate
8690
- [ ] On deserialization failure, print the raw bencoded payload as generic JSON (via
8791
`bencode2json`)
8892
- [ ] If `bencode2json` conversion also fails, print a warning with the raw byte slice
8993
- [ ] The process exits with a non-zero exit code when the response cannot be deserialized
9094
(print the fallback JSON/bytes to stdout, return an `Err` from the command function)
95+
- [ ] Fallback JSON output is compact by default in this issue; once `--format`
96+
is introduced in #1562, fallback JSON must respect the selected format
9197
- [ ] `linter all` exits with code `0`
9298
- [ ] `cargo machete` reports no unused dependencies
9399
- [ ] All existing tests pass
@@ -109,6 +115,9 @@ pub fn try_from_bencoded(bytes: &[u8]) -> Result<Self, BencodeParseError> {
109115

110116
A new `BencodeParseError` variant may be needed for `serde_bencode::Error`.
111117

118+
Also replace nested `.unwrap()` calls in scrape parsing helpers with proper
119+
error propagation into `BencodeParseError`.
120+
112121
### Task 2: Add `bencode2json` dependency
113122

114123
In `console/tracker-client/Cargo.toml`, add:
@@ -169,6 +178,7 @@ Add examples showing the fallback output in the module-level doc comment.
169178
- [ ] Running the client against the Torrust Tracker still prints the typed JSON response
170179
and exits `0`
171180
- [ ] No `panic!` or `.unwrap()` in the announce or scrape command paths
181+
- [ ] No reachable panic/unwrap remains in the scrape decoding path
172182
- [ ] `linter all` exits with code `0`
173183
- [ ] `cargo machete` reports no unused dependencies
174184
- [ ] All existing tests pass

project-words.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ mysqld
167167
Naim
168168
nanos
169169
newkey
170+
newtrackon
170171
newtype
171172
newtypes
172173
nextest

0 commit comments

Comments
 (0)