diff --git a/docs/issues/1525-07-align-rust-and-db-types.md b/docs/issues/1525-07-align-rust-and-db-types.md deleted file mode 100644 index 240d1671a..000000000 --- a/docs/issues/1525-07-align-rust-and-db-types.md +++ /dev/null @@ -1,228 +0,0 @@ -# Subissue 1525-07: Align Rust and Database Types - -## Goal - -Widen the download-counter type in Rust from `u32` to `u64` and widen the corresponding -database columns from `INTEGER` (32-bit, MySQL) to `BIGINT` (64-bit), delivered as a versioned -`sqlx` migration so the change is explicit, testable, and tracked as a forward schema change. - -## Background - -### Current state - -By the time this subissue is implemented, subissue `1525-06` will have wired `sqlx::migrate!()` -into both drivers. The schema at that point contains: - -- `torrents.completed` — `INTEGER` in MySQL (32-bit signed, max ≈ 2.1 billion), `INTEGER` in - SQLite (storage is already 64-bit for any integer value). -- `torrent_aggregate_metrics.value` — same types as above. - -The Rust type alias is `NumberOfDownloads = u32` in -`packages/primitives/src/lib.rs`. The `SwarmMetadata.downloaded` field also uses this type. -The drivers read the column as `i64` (sqlx always returns integer columns as `i64`) and -immediately narrow-cast to `u32`. - -### Why this is a problem - -The MySQL `INT` column type is **signed 32-bit** (max 2,147,483,647). Writing a `u32` value -above that limit silently overflows or errors. Practically, the counter saturates at the same -point as the UDP scrape wire format (`completed` is `i32` in BEP 15), but the correct fix is -to widen the storage type rather than rely on implicit saturation in the driver. - -`u32::MAX` (4,294,967,295) is already higher than the `i32::MAX` wire limit, so protocol -saturation happens before storage overflow today. However, aligning storage to `BIGINT` and the -Rust type to `u64` makes the storage contract explicit and decoupled from any particular -protocol encoding. Future protocol changes or a direct-database query tool cannot accidentally -exceed a silently-constrained column. - -**Protocol encoding** (read-only, no changes needed in this subissue): - -- UDP scrape response (`i32` wire field): the existing conversion from `NumberOfDownloads` to - `i32` already saturates at `i32::MAX`. This remains unchanged. -- HTTP scrape response (bencoded `i64`): `bencode_download_count()` saturates at `i64::MAX`. - This remains unchanged. - -### Why migrations first (1525-06 before 1525-07) - -The column-widening change must be delivered as a versioned migration rather than an ad hoc DDL -update. Having the migration framework from `1525-06` in place ensures the change is tracked in -`_sqlx_migrations`, tested like any other migration, and can be reasoned about in production -upgrade scenarios. - -## Proposed Branch - -- `1525-07-align-rust-and-db-types` - -## What Changes - -### Migration files - -Add the fourth migration to both existing backends: - -```text -packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql -packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql -``` - -**SQLite** — no-op (SQLite already stores any `INTEGER` value as a 64-bit signed integer): - -```sql --- SQLite stores INTEGER values as signed 64-bit integers already. --- This migration is intentionally a no-op so the migration history stays --- aligned with the MySQL backend. -``` - -**MySQL** — widen both download-counter columns: - -```sql -ALTER TABLE torrents - MODIFY completed BIGINT NOT NULL DEFAULT 0; - -ALTER TABLE torrent_aggregate_metrics - MODIFY value BIGINT NOT NULL DEFAULT 0; -``` - -PostgreSQL migration files are not created here. They will be added in subissue `1525-08` when -the PostgreSQL driver is introduced. Following the -[history-alignment pattern](1719-1525-06-introduce-schema-migrations.md#history-alignment-pattern) -established in `1525-06`, subissue `1525-08` creates **all four** migration files for -PostgreSQL starting from migration 1. PostgreSQL's migration 1 creates the columns as -`INTEGER` (matching the original schema from the other backends), and migration 4 widens them -to `BIGINT` using PostgreSQL-specific `ALTER COLUMN ... TYPE BIGINT` syntax. Migration 4 is -not a no-op for PostgreSQL. - -### Rust type changes - -**`packages/primitives/src/lib.rs`** — widen the type alias: - -```rust -// Before -pub type NumberOfDownloads = u32; - -// After -pub type NumberOfDownloads = u64; -``` - -**`packages/primitives/src/swarm_metadata.rs`** — `downloaded` field currently uses the bare -`u32`. Update it to use `NumberOfDownloads` explicitly: - -```rust -// Before -pub downloaded: u32, - -// After -pub downloaded: NumberOfDownloads, -``` - -Also update the `downloads()` method return type to `NumberOfDownloads`. - -### Driver conversion changes - -After `1525-05`, the sqlx drivers read counter columns as `i64`. With `NumberOfDownloads = u32` -the read path does `u32::try_from(i64_value)`. After this subissue it becomes -`u64::try_from(i64_value)`. - -Because the database column type is `BIGINT` (signed), the **write path** must also encode -`u64 → i64`. Values above `i64::MAX` (≈ 9.2 × 10¹⁸) cannot be stored and must return an -error rather than silently truncate. Add named helper methods to each driver to make the -conversion explicit and consistent: - -```rust -fn decode_counter(value: i64) -> Result { - u64::try_from(value).map_err(|err| Error::invalid_query(DRIVER, err)) -} - -fn encode_counter(value: NumberOfDownloads) -> Result { - i64::try_from(value).map_err(|err| Error::invalid_query(DRIVER, err)) -} -``` - -Use these helpers in every place a counter column is read from or written to the database. - -### Cascade compilation fixes - -Widening `NumberOfDownloads` from `u32` to `u64` will produce compilation errors wherever the -old `u32` range was assumed. Fix all errors; do not add `as u32` casts or `allow` attributes -to suppress them. - -## Tasks - -### Task 1 — Add migration files - -Create the two new migration files listed above. Do not modify any existing migration file. - -**Outcome**: `packages/tracker-core/migrations/` has four files in each of `sqlite/` and -`mysql/`. The fourth file is verified by running the migration against a fresh test database -of each type. - -### Task 2 — Widen `NumberOfDownloads` and fix cascade - -Change `NumberOfDownloads = u32 → u64` in `packages/primitives/src/lib.rs` and update -`SwarmMetadata.downloaded` to use the alias. Fix all resulting compilation errors across the -workspace (driver conversion logic, scrape response encoding, announce handler arithmetic, -etc.). - -Add `decode_counter` / `encode_counter` helpers to both driver files as described above. - -**Outcome**: `cargo build --workspace` succeeds with no warnings or errors. - -### Task 3 — Validate migration and type alignment - -Add or extend tests that verify: - -- **MySQL migration**: running the migration on a database with the pre-migration `INT` column - produces a `BIGINT` column, and writing and reading a value larger than `2^31 − 1` round-trips - correctly. -- **SQLite no-op**: the migration applies cleanly (recorded in `_sqlx_migrations`) and the - column already accepts large values. -- **Boundary encode**: writing a `u64` counter value of exactly `i64::MAX` succeeds; writing - `i64::MAX + 1` returns an appropriate error rather than panicking or wrapping. - -These tests extend the existing driver `#[cfg(test)]` modules. - -**Outcome**: `cargo test --workspace --all-targets` passes. - -## Out of Scope - -- PostgreSQL migration files — added in subissue `1525-08`. -- Down migrations (rollback) — not needed at this stage. -- Trait splitting or other structural refactoring. -- Other numeric types beyond `NumberOfDownloads` / download counters. - -## Acceptance Criteria - -- [ ] `packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql` - exists and is a comment-only no-op. -- [ ] `packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql` - exists and widens `torrents.completed` and `torrent_aggregate_metrics.value` to `BIGINT`. -- [ ] `NumberOfDownloads = u64` in `packages/primitives/src/lib.rs`. -- [ ] `SwarmMetadata.downloaded` uses `NumberOfDownloads`; bare `u32` is removed from that field. -- [ ] Both driver files use explicit `decode_counter` / `encode_counter` helpers for all - counter-column reads and writes. -- [ ] `encode_counter` returns an error (not a panic, not silent truncation) for values - above `i64::MAX`. -- [ ] A test verifies round-trip of a value larger than `u32::MAX` for each backend. -- [ ] A test verifies the encode error path for values above `i64::MAX`. -- [ ] No `as u32` casts or compiler-suppression attributes introduced by this subissue. -- [ ] Persistence benchmarking (see subissue `1525-03`) shows no regression against the - committed baseline. -- [ ] `cargo test --workspace --all-targets` passes. -- [ ] `linter all` exits with code `0`. - -## References - -- EPIC: `#1525` -- Subissue `1525-06`: `docs/issues/1719-1525-06-introduce-schema-migrations.md` — must be completed - first (provides the migration framework) -- Subissue `1525-08`: `docs/issues/1525-08-add-postgresql-driver.md` — adds PostgreSQL - migration files including the history-aligned no-op for this migration -- Subissue `1525-03`: `docs/issues/1525-03-persistence-benchmarking.md` — benchmark baseline -- Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout - instructions (`docs/issues/1525-overhaul-persistence.md`) -- Reference files: - - `packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql` - - `packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql` - - `packages/primitives/src/lib.rs` (type alias change) - - `packages/primitives/src/swarm_metadata.rs` (field type change) - - `packages/tracker-core/src/databases/driver/sqlite.rs` (decode/encode helpers) - - `packages/tracker-core/src/databases/driver/mysql.rs` (decode/encode helpers) diff --git a/docs/issues/1525-08-add-postgresql-driver.md b/docs/issues/1525-08-add-postgresql-driver.md index 0ee539659..71407fbbe 100644 --- a/docs/issues/1525-08-add-postgresql-driver.md +++ b/docs/issues/1525-08-add-postgresql-driver.md @@ -38,9 +38,9 @@ By the time this subissue is implemented: a `bootstrap_legacy_schema()` helper for upgrading pre-v4 databases. Both backends have three migration files. -- **1525-07** has widened `NumberOfDownloads` from `u32` to `u64`, added a fourth migration to - SQLite and MySQL, and added `decode_counter`/`encode_counter` helpers to both drivers. The - migration file layout at the end of `1525-07` is: +- **1525-07** has widened MySQL download-counter columns to `BIGINT` via a fourth migration, + added a history-aligned no-op migration for SQLite, and kept `NumberOfDownloads = u32`. + The migration file layout at the end of `1525-07` is: ```text packages/tracker-core/migrations/ @@ -323,7 +323,7 @@ async fn drop_database_tables(&self) -> Result<(), Error> { ```rust fn decode_counter(value: i64) -> Result { - u64::try_from(value).map_err(|err| Error::invalid_query(DRIVER, err)) + u32::try_from(value).map_err(|err| Error::invalid_query(DRIVER, err)) } fn encode_counter(value: NumberOfDownloads) -> Result { @@ -332,7 +332,7 @@ fn encode_counter(value: NumberOfDownloads) -> Result { ``` Use these helpers in every place a counter column is read from or written to the database. -Do not use bare `as i64` casts or `as u64` casts. +Do not use bare `as i64` casts or `as u32` casts. **`TorrentMetricsStore`, `WhitelistStore`, `AuthKeyStore` implementations**: Follow the same structure as the SQLite and MySQL drivers, substituting `$1`/`$2` placeholders and the @@ -482,7 +482,7 @@ Acceptance criteria: - [ ] `create_database_tables()` calls `MIGRATOR.run()` with no legacy bootstrap. - [ ] `drop_database_tables()` drops all five tables including `_sqlx_migrations`. - [ ] All counter reads use `decode_counter`; all counter writes use `encode_counter`. -- [ ] No bare `as i64` or `as u64` casts in the driver. +- [ ] No bare `as i64` or `as u32` casts in the driver. ### Task 4 — Wire the PostgreSQL driver into the factory and setup @@ -773,8 +773,8 @@ Acceptance criteria: (PostgreSQL deferred here) - Subissue `1525-06`: `docs/issues/1719-1525-06-introduce-schema-migrations.md` — migration framework and history-alignment pattern -- Subissue `1525-07`: `docs/issues/1525-07-align-rust-and-db-types.md` — fourth migration - and `NumberOfDownloads = u64` +- Subissue `1525-07`: `docs/issues/1721-1525-07-align-rust-and-db-types.md` — fourth migration + and DB-only widening (`NumberOfDownloads = u32`) - Reference PR: `#1695` - Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout instructions diff --git a/docs/issues/1525-overhaul-persistence.md b/docs/issues/1525-overhaul-persistence.md index 474185d65..25fb2ec53 100644 --- a/docs/issues/1525-overhaul-persistence.md +++ b/docs/issues/1525-overhaul-persistence.md @@ -124,7 +124,7 @@ You can then browse or search it while working in the main repository. ### 7) Align persisted counters and Rust/SQL type boundaries -- Spec file: `docs/issues/1525-07-align-rust-and-db-types.md` +- Spec file: `docs/issues/1721-1525-07-align-rust-and-db-types.md` - Outcome: explicit contract for persisted counters and numeric ranges, with any needed schema changes delivered through migrations diff --git a/docs/issues/1721-1525-07-align-rust-and-db-types.md b/docs/issues/1721-1525-07-align-rust-and-db-types.md new file mode 100644 index 000000000..97614c104 --- /dev/null +++ b/docs/issues/1721-1525-07-align-rust-and-db-types.md @@ -0,0 +1,254 @@ +# Subissue 1525-07: Align Rust and Database Types + +## Goal + +Widen the MySQL download-counter columns from `INTEGER` (32-bit signed) to `BIGINT` (64-bit), +delivered as a versioned `sqlx` migration. The Rust type `NumberOfDownloads` stays `u32` — +the database column is intentionally wider than the Rust type, and that is the correct design +(see [Design Decision](#design-decision-widen-db-only-keep-rust-type) below). + +## Type-Mapping Diagram + +### Current state (before this subissue) + +```text +DB column (MySQL) sqlx read Driver cast Rust domain Wire (write) +──────────────────── ────────── ──────────── ───────────── ────────────────────── +torrents.completed + INT (signed 32-bit) → i64 → u32::try_from NumberOfDownloads UDP: i32::try_from (saturate) + max 2,147,483,647 (may error!) = u32 HTTP: i64::from(u32) (infallible) + +torrent_aggregate_metrics.value + INT (signed 32-bit) → i64 → u32::try_from (same alias) + max 2,147,483,647 (may error!) +``` + +**Problem**: `u32::MAX` (4,294,967,295) > `i32::MAX` (2,147,483,647). Once the counter exceeds +`i32::MAX`, the MySQL write fails or overflows silently. + +### Final state (after this subissue) + +```text +DB column (MySQL) sqlx read Driver cast Rust domain Wire (write) +──────────────────── ────────── ──────────── ───────────── ────────────────────── +torrents.completed + BIGINT (signed 64) → i64 → u32::try_from NumberOfDownloads UDP: i32::try_from (saturate) + max 9,223,372,036,… (infallible = u32 HTTP: i64::from(u32) (infallible) + for u32 range) + +torrent_aggregate_metrics.value + BIGINT (signed 64) → i64 → u32::try_from (same alias) + max 9,223,372,036,… (infallible + for u32 range) +``` + +**SQLite**: no column change needed — SQLite `INTEGER` already stores any value as signed +64-bit. A no-op migration is added solely to keep the migration history aligned with MySQL. + +## Background + +### Current state + +By the time this subissue is implemented, subissue `1525-06` will have wired `sqlx::migrate!()` +into both drivers. The schema at that point contains: + +- `torrents.completed` — `INTEGER` in MySQL (32-bit signed, max ≈ 2.1 billion), `INTEGER` in + SQLite (storage is already 64-bit for any integer value). +- `torrent_aggregate_metrics.value` — same types as above. + +The Rust type alias is `NumberOfDownloads = u32` in +`packages/primitives/src/lib.rs`. The `SwarmMetadata.downloaded` field also uses this type. +The drivers read the column as `i64` (sqlx always returns integer columns as `i64`) and +narrow-cast to `u32`. + +### Why this is a problem + +The MySQL `INT` column type is **signed 32-bit** (max 2,147,483,647). `u32::MAX` is +4,294,967,295 — roughly double that limit. Once the download counter exceeds `i32::MAX` the +MySQL write fails or silently overflows. Widening the column to `BIGINT` removes this ceiling +while keeping the Rust type and all existing wire-encoding logic unchanged. + +**Protocol encoding** (no changes in this subissue): + +- UDP scrape (`i32` wire field): `i32::try_from(u32)` already saturates at `i32::MAX`. +- HTTP scrape (bencoded `i64`): `i64::from(u32)` is infallible; no change needed. + +### Why migrations first (1525-06 before 1525-07) + +The column-widening change must be a versioned migration, not ad hoc DDL. The migration +framework from `1525-06` ensures the change is recorded in `_sqlx_migrations`, testable, and +safe in production upgrade scenarios. + +## Design Decision: Widen DB Only, Keep Rust Type + +The initial proposal for this subissue suggested widening `NumberOfDownloads` from `u32` to +`u64` alongside the database column. After analysis, **only the DB column is widened**. The +Rust type stays `u32`. Here is the reasoning: + +### Why NOT widen the Rust type + +The database in this tracker is an internal persistence store, not a shared external system. +No other service writes to it directly. Writing a value above `u32::MAX` into this database +would mean the application logic itself had produced that value — which is impossible while +`NumberOfDownloads = u32`. The write path is therefore fully bounded by the Rust type at +compile time. + +This is the same reasoning as storing an enum variant as a string in the database: the string +column could hold arbitrary text, but the application only ever writes valid variant names. The +wider storage type is intentional; it does not indicate that the application type should match it. + +### The read path is safe too + +If someone bypassed the application and wrote a value above `u32::MAX` directly into the +database, the driver would return a `MalformedDatabaseRecord` error at read time — which is the +correct behaviour. The application should not silently accept data that violates its own +invariants. We already have similar guarded conversions elsewhere in the drivers. + +### Why the original proposal suggested `u64` + +The original motivation was defensive: aligning the Rust type to the full BIGINT range would +make the read path infallible and future-proof against protocol changes. That reasoning is +valid, but it comes at the cost of a large cascade change (scrape encoders, swarm metadata, +benchmark helpers, UDP handler) for a scenario — direct external writes — that is out of scope +and would break other invariants anyway. The simpler approach (widen DB only) fixes the actual +bug with minimal churn. + +### `SwarmMetadata` field types + +`complete` and `incomplete` in `SwarmMetadata` are point-in-time counts of currently connected +seeders and leechers. They are in-memory only and never persisted. Widening them would add +scope without fixing any real problem; they remain `u32`. + +`downloaded` is the persisted accumulator. It stays `u32` in Rust but the field should use the +`NumberOfDownloads` type alias (not the bare `u32`) to make the intent explicit. This is a +cosmetic fix included in Task 2. + +## Proposed Branch + +- `1721-1525-07-align-rust-and-db-types` + +## What Changes + +### Migration files + +Add the fourth migration to both existing backends: + +```text +packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql +packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql +``` + +**SQLite** — no-op (SQLite already stores any `INTEGER` value as a 64-bit signed integer): + +```sql +-- SQLite stores INTEGER values as signed 64-bit integers already. +-- This migration is intentionally a no-op so the migration history stays +-- aligned with the MySQL backend. +``` + +**MySQL** — widen both download-counter columns: + +```sql +ALTER TABLE torrents + MODIFY completed BIGINT NOT NULL DEFAULT 0; + +ALTER TABLE torrent_aggregate_metrics + MODIFY value BIGINT NOT NULL DEFAULT 0; +``` + +PostgreSQL migration files are not created here. They will be added in subissue `1525-08` when +the PostgreSQL driver is introduced. Following the +[history-alignment pattern](1719-1525-06-introduce-schema-migrations.md#history-alignment-pattern) +established in `1525-06`, subissue `1525-08` creates **all four** migration files for +PostgreSQL starting from migration 1. PostgreSQL's migration 4 widens the columns using +PostgreSQL-specific `ALTER COLUMN ... TYPE BIGINT` syntax; it is not a no-op for PostgreSQL. + +### Rust changes (cosmetic only) + +**`packages/primitives/src/swarm_metadata.rs`** — use the `NumberOfDownloads` alias instead +of the bare `u32` for the `downloaded` field and the `downloads()` return type: + +```rust +// Before +pub downloaded: u32, +pub fn downloads(&self) -> u32 { ... } + +// After +pub downloaded: NumberOfDownloads, +pub fn downloads(&self) -> NumberOfDownloads { ... } +``` + +`NumberOfDownloads` remains `u32` in `packages/primitives/src/lib.rs`. No other Rust types +change. No cascade compilation fixes are required. + +## Tasks + +### Task 1 — Add migration files + +Create the two new migration files listed above. Do not modify any existing migration file. + +**Outcome**: `packages/tracker-core/migrations/` has four files in each of `sqlite/` and +`mysql/`. The fourth file is verified by running the migration against a fresh test database +of each type. + +### Task 2 — Use `NumberOfDownloads` alias in `SwarmMetadata` + +Update `SwarmMetadata.downloaded` and `downloads()` to use the `NumberOfDownloads` alias +instead of the bare `u32`. This is a cosmetic change; no logic changes. + +**Outcome**: `cargo build --workspace` succeeds with no warnings or errors. + +### Task 3 — Validate the migration + +Add or extend tests that verify: + +- **MySQL migration**: running the migration on a database with the pre-migration `INT` column + produces a `BIGINT` column, and writing and reading a value in the range `(i32::MAX, u32::MAX]` + round-trips correctly (this range was previously unsafe with `INT`). +- **SQLite no-op**: the migration applies cleanly (recorded in `_sqlx_migrations`) and the + column continues to accept all values in the `u32` range. + +These tests extend the existing driver `#[cfg(test)]` modules. + +**Outcome**: `cargo test --workspace --all-targets` passes. + +## Out of Scope + +- Widening `NumberOfDownloads` to `u64` — explicitly out of scope (see Design Decision above). +- PostgreSQL migration files — added in subissue `1525-08`. +- Down migrations (rollback) — not needed at this stage. +- Trait splitting or other structural refactoring. +- Changes to `complete` / `incomplete` fields in `SwarmMetadata`. + +## Acceptance Criteria + +- [ ] `packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql` + exists and is a comment-only no-op. +- [ ] `packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql` + exists and widens `torrents.completed` and `torrent_aggregate_metrics.value` to `BIGINT`. +- [ ] `NumberOfDownloads` remains `u32` in `packages/primitives/src/lib.rs`. +- [ ] `SwarmMetadata.downloaded` and `downloads()` use the `NumberOfDownloads` alias; bare + `u32` is replaced with the alias in that struct. +- [ ] A test verifies that writing and reading a value in `(i32::MAX, u32::MAX]` round-trips + correctly on MySQL after the migration. +- [ ] A test verifies the SQLite no-op migration applies cleanly. +- [ ] No new `as u32` casts or compiler-suppression attributes introduced by this subissue. +- [ ] Persistence benchmarking (see subissue `1525-03`) shows no regression against the + committed baseline. +- [ ] `cargo test --workspace --all-targets` passes. +- [ ] `linter all` exits with code `0`. + +## References + +- EPIC: `#1525` +- Subissue `1525-06`: `docs/issues/1719-1525-06-introduce-schema-migrations.md` — must be completed + first (provides the migration framework) +- Subissue `1525-08`: `docs/issues/1525-08-add-postgresql-driver.md` — adds PostgreSQL + migration files including the history-aligned no-op for this migration +- Subissue `1525-03`: `docs/issues/1525-03-persistence-benchmarking.md` — benchmark baseline +- Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout + instructions (`docs/issues/1525-overhaul-persistence.md`) +- Reference files: + - `packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql` + - `packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql` + - `packages/primitives/src/swarm_metadata.rs` (alias cosmetic fix) diff --git a/packages/primitives/src/swarm_metadata.rs b/packages/primitives/src/swarm_metadata.rs index 57ba816d3..d4edeff81 100644 --- a/packages/primitives/src/swarm_metadata.rs +++ b/packages/primitives/src/swarm_metadata.rs @@ -2,6 +2,8 @@ use std::ops::AddAssign; use derive_more::Constructor; +use crate::NumberOfDownloads; + /// Swarm statistics for one torrent. /// /// Swarm metadata dictionary in the scrape response. @@ -11,7 +13,7 @@ use derive_more::Constructor; pub struct SwarmMetadata { /// (i.e `completed`): The number of peers that have ever completed /// downloading a given torrent. - pub downloaded: u32, + pub downloaded: NumberOfDownloads, /// (i.e `seeders`): The number of active peers that have completed /// downloading (seeders) a given torrent. @@ -29,7 +31,7 @@ impl SwarmMetadata { } #[must_use] - pub fn downloads(&self) -> u32 { + pub fn downloads(&self) -> NumberOfDownloads { self.downloaded } diff --git a/packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql b/packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql new file mode 100644 index 000000000..ae0e48dec --- /dev/null +++ b/packages/tracker-core/migrations/mysql/20260409120000_torrust_tracker_widen_download_counters.sql @@ -0,0 +1,3 @@ +ALTER TABLE torrents MODIFY completed BIGINT NOT NULL DEFAULT 0; + +ALTER TABLE torrent_aggregate_metrics MODIFY value BIGINT NOT NULL DEFAULT 0; \ No newline at end of file diff --git a/packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql b/packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql new file mode 100644 index 000000000..7a77cd86b --- /dev/null +++ b/packages/tracker-core/migrations/sqlite/20260409120000_torrust_tracker_widen_download_counters.sql @@ -0,0 +1,3 @@ +-- SQLite stores INTEGER values as signed 64-bit integers already. +-- This migration is intentionally a no-op so the migration history stays +-- aligned with the MySQL backend. \ No newline at end of file diff --git a/packages/tracker-core/src/databases/driver/mysql/mod.rs b/packages/tracker-core/src/databases/driver/mysql/mod.rs index 3f9c97729..1af4aaef9 100644 --- a/packages/tracker-core/src/databases/driver/mysql/mod.rs +++ b/packages/tracker-core/src/databases/driver/mysql/mod.rs @@ -103,6 +103,7 @@ mod tests { use super::Mysql; use crate::databases::driver::tests::run_tests; use crate::databases::traits::Database; + use crate::test_helpers::tests::random_info_hash; #[derive(Debug, Default)] struct StoppedMysqlContainer {} @@ -241,7 +242,36 @@ mod tests { .fetch_one(&raw_pool) .await .expect("count _sqlx_migrations"); - assert_eq!(recorded, 3, "all three legacy migrations should be fake-applied"); + assert_eq!( + recorded, 4, + "all migrations should be recorded after bootstrap + migrator run" + ); + + assert_mysql_column_type(&raw_pool, "torrents", "completed", "bigint").await; + assert_mysql_column_type(&raw_pool, "torrent_aggregate_metrics", "value", "bigint").await; + + let above_i32_max = 2_200_000_000_u32; + let info_hash = random_info_hash(); + + driver + .save_torrent_downloads(&info_hash, above_i32_max) + .await + .expect("save torrent downloads above i32::MAX should succeed"); + let loaded_torrent_downloads = driver + .load_torrent_downloads(&info_hash) + .await + .expect("load torrent downloads above i32::MAX should succeed"); + assert_eq!(loaded_torrent_downloads, Some(above_i32_max)); + + driver + .save_global_downloads(above_i32_max) + .await + .expect("save global downloads above i32::MAX should succeed"); + let loaded_global_downloads = driver + .load_global_downloads() + .await + .expect("load global downloads above i32::MAX should succeed"); + assert_eq!(loaded_global_downloads, Some(above_i32_max)); // Partial-state rejection: only two of four legacy tables present. driver @@ -297,4 +327,19 @@ mod tests { ::sqlx::query(stmt).execute(pool).await.expect("legacy DDL"); } } + + async fn assert_mysql_column_type(pool: &::sqlx::MySqlPool, table: &str, column: &str, expected_type: &str) { + let data_type_bytes: Vec = ::sqlx::query_scalar( + "SELECT DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ? AND COLUMN_NAME = ?", + ) + .bind(table) + .bind(column) + .fetch_one(pool) + .await + .expect("column type query should succeed"); + + let data_type = String::from_utf8_lossy(&data_type_bytes).to_lowercase(); + + assert_eq!(data_type, expected_type, "{table}.{column} should be {expected_type}"); + } } diff --git a/packages/tracker-core/src/databases/driver/sqlite/mod.rs b/packages/tracker-core/src/databases/driver/sqlite/mod.rs index 422e99340..a79794c81 100644 --- a/packages/tracker-core/src/databases/driver/sqlite/mod.rs +++ b/packages/tracker-core/src/databases/driver/sqlite/mod.rs @@ -117,6 +117,13 @@ mod tests { async fn create_database_tables_should_be_idempotent_on_a_fresh_database() { let config = ephemeral_configuration(); let driver = initialize_driver(&config); + let options = ::sqlx::sqlite::SqliteConnectOptions::new() + .filename(&config.database.path) + .create_if_missing(true); + let pool = ::sqlx::sqlite::SqlitePoolOptions::new() + .connect_with(options) + .await + .expect("connect sqlite for migration count"); // First call applies every embedded migration. driver @@ -130,5 +137,11 @@ mod tests { .create_database_tables() .await .expect("second migration run should be a no-op"); + + let recorded: i64 = ::sqlx::query_scalar("SELECT COUNT(*) FROM _sqlx_migrations") + .fetch_one(&pool) + .await + .expect("count _sqlx_migrations"); + assert_eq!(recorded, 4, "all four migrations should be recorded"); } }