feat(tracker-core): split Database trait into four narrow persistence traits

docs/adrs/20260429000000_keep_database_as_aggregate_supertrait.md

@@ -0,0 +1,94 @@
+# Keep `Database` as an Aggregate Supertrait
+
+## Description
+
+The persistence layer used a single monolithic `Database` trait with 18 methods
+spanning four distinct concerns: schema lifecycle, torrent metrics, whitelist
+management, and authentication keys. Consumers that only needed one concern
+(e.g. `DatabaseKeyRepository`) were forced to depend on the full 18-method
+interface, making tests harder to write and clouding the intent of each service.
+
+The question was how to split the trait while preserving a single, discoverable
+contract that all database drivers must satisfy.
+
+## Agreement
+
+Split `Database` into four narrow context traits:
+
+- `SchemaMigrator` — `create_database_tables`, `drop_database_tables`
+- `TorrentMetricsStore` — load/save/increase per-torrent and global download counters (7 methods)
+- `WhitelistStore` — load/get/add/remove infohash whitelist entries (4 required + 1 default method)
+- `AuthKeyStore` — load/get/add/remove authentication keys (4 methods)
+
+Keep `Database` as an **empty aggregate supertrait** with a blanket implementation:
+
+```rust
+pub trait Database: Sync + Send + SchemaMigrator + TorrentMetricsStore + WhitelistStore + AuthKeyStore {}
+
+impl<T> Database for T where T: Sync + Send + SchemaMigrator + TorrentMetricsStore + WhitelistStore + AuthKeyStore {}
+```
+
+`Database` is a **private, internal compile-time contract** for driver
+completeness only. External consumers (services, repositories, tests) will
+progress toward using only the narrow traits they actually need. That migration
+happens in future subissues and does not require changing any consumer in this
+step.
+
+### Alternatives Considered
+
+**Independent traits only (no `Database` supertrait)** — Each driver would
+implement four separate traits; consumers would receive `Arc<Box<dyn AuthKeyStore>>`
+etc. instead of `Arc<Box<dyn Database>>`.
+
+Rejected because:
+
+1. There would be no single place to verify that a driver implements the
+   complete persistence contract — the compiler can no longer catch a partially
+   implemented driver as one unit.
+2. Changing every call site (container wiring, factory, tests) all at once
+   would turn this structural step into a much larger, riskier diff. The
+   aggregate supertrait lets the split land cleanly first; consumer migration
+   follows in subsequent subissues.
+
+Note on trait-object upcasting: migrating consumers to narrow traits does **not**
+require upcasting (`dyn Database` → `dyn WhitelistStore`). The factory will
+construct the concrete driver type (e.g. `Arc<Sqlite>`) and coerce it directly
+into each narrow trait object (`Arc<dyn WhitelistStore>`, etc.). Coercion from
+a sized type to a trait object is available on all Rust versions; upcasting
+between two trait objects would be a different story, but is not needed here.
+
+### Consequences
+
+#### Positive
+
+- Each narrow trait expresses a single context; services and tests can depend
+  only on the interface they actually need.
+- `#[automock]` on each narrow trait generates focused mocks (`MockAuthKeyStore`
+  etc.) instead of one 18-method mega-mock.
+- The blanket impl makes it impossible to partially implement `Database`:
+  the compiler enforces completeness of all four narrow traits together.
+
+### Negative
+
+- Tests that previously used `MockDatabase` must be updated to use the
+  appropriate narrow mock (`MockWhitelistStore`, `MockAuthKeyStore`, etc.).
+  This is actually simpler — each mock covers only the methods the test cares
+  about — but it is a mechanical change across test files.
+- `Database` will persist as long as `Arc<Box<dyn Database>>` wiring exists.
+  That wiring will be replaced in subissue #1525-04b
+  ([docs/issues/1525-04b-migrate-consumers-to-narrow-traits.md](../issues/1525-04b-migrate-consumers-to-narrow-traits.md))
+  by a plain `DatabaseStores` struct (one `Arc<dyn XxxStore>` field per
+  context). `TrackerCoreContainer` will hold `DatabaseStores` instead of
+  `Arc<Box<dyn Database>>`; each service is wired at construction time by
+  passing only the narrow store it needs. At that point `Database` can be
+  made fully private or removed.
+
+## Date
+
+2026-04-29
+
+## References
+
+- Issue spec: [docs/issues/1713-1525-04-split-persistence-traits.md](../issues/1713-1525-04-split-persistence-traits.md)
+- GitHub issue: <https://github.com/torrust/torrust-tracker/issues/1713>
+- EPIC: [docs/issues/1525-overhaul-persistence.md](../issues/1525-overhaul-persistence.md)

docs/adrs/index.md

@@ -1,6 +1,7 @@
 # ADR Index
 
-| ADR                                                                                     | Date       | Title                                                  | Short Description                                                                                                              |
-| --------------------------------------------------------------------------------------- | ---------- | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------ |
-| [20240227164834](20240227164834_use_plural_for_modules_containing_collections.md)       | 2024-02-27 | Use plural for modules containing collections          | Module names should use plural when they contain multiple types with the same responsibility (e.g. `requests/`, `responses/`). |
-| [20260420200013](20260420200013_adopt_custom_github_copilot_aligned_agent_framework.md) | 2026-04-20 | Adopt a custom, GitHub-Copilot-aligned agent framework | Use AGENTS.md, Agent Skills, and Custom Agent profiles instead of third-party agent frameworks.                                |
+| ADR                                                                                     | Date       | Title                                                  | Short Description                                                                                                                                                                                                                          |
+| --------------------------------------------------------------------------------------- | ---------- | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+| [20240227164834](20240227164834_use_plural_for_modules_containing_collections.md)       | 2024-02-27 | Use plural for modules containing collections          | Module names should use plural when they contain multiple types with the same responsibility (e.g. `requests/`, `responses/`).                                                                                                             |
+| [20260420200013](20260420200013_adopt_custom_github_copilot_aligned_agent_framework.md) | 2026-04-20 | Adopt a custom, GitHub-Copilot-aligned agent framework | Use AGENTS.md, Agent Skills, and Custom Agent profiles instead of third-party agent frameworks.                                                                                                                                            |
+| [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. |

docs/issues/1525-04b-migrate-consumers-to-narrow-traits.md

@@ -0,0 +1,170 @@
+# Subissue Draft for #1525-04b: Migrate Consumers to Narrow Persistence Traits
+
+## Goal
+
+Replace every use of `Arc<Box<dyn Database>>` in production and test code with
+the specific narrow trait the consumer actually needs (`AuthKeyStore`,
+`TorrentMetricsStore`, `WhitelistStore`, or `SchemaMigrator`). After this
+subissue the `Database` aggregate supertrait becomes a purely internal
+compile-time guard that is no longer part of the public surface of
+`tracker-core`.
+
+## Background
+
+Subissue #1525-04 (GitHub [#1713](https://github.com/torrust/torrust-tracker/issues/1713))
+introduced the four narrow traits and kept `Database` as an aggregate supertrait
+so that consumer call sites did not need to change.
+
+Now that the structural split is in place, this subissue wires consumers to the
+narrow traits they actually need. No upcasting is required: the factory will
+construct the concrete driver (`Sqlite`, `Mysql`) and coerce it directly into
+each narrow `Arc<dyn XxxStore>`. Coercion from a sized type to a trait object is
+available on all Rust versions.
+
+## Proposed Branch
+
+- `1525-04b-migrate-consumers-to-narrow-traits`
+
+## Current State
+
+All consumers depend on `Arc<Box<dyn Database>>` for everything, regardless of
+which methods they actually call:
+
+| Consumer                                           | Methods actually used                                       |
+| -------------------------------------------------- | ----------------------------------------------------------- |
+| `DatabaseKeyRepository`                            | `AuthKeyStore` methods only                                 |
+| `DatabaseDownloadsMetricRepository`                | `TorrentMetricsStore` methods only                          |
+| `whitelist::setup::initialize_whitelist_manager`   | `WhitelistStore` methods only                               |
+| `databases::driver::build` / `initialize_database` | `SchemaMigrator::create_database_tables` only               |
+| `bin/persistence_benchmark`                        | All four concerns — uses `Database` as a convenience bundle |
+| `container::TrackerCoreContainer`                  | Holds the database and fans it out to the above             |
+
+## Target State
+
+```text
+TrackerCoreContainer
+  database_stores:        DatabaseStores   ← replaces Arc<Box<dyn Database>>
+  ...rest of fields unchanged...
+```
+
+`DatabaseStores` is a plain struct holding one `Arc<dyn XxxStore>` per context.
+The container stores it as one named field; individual services are wired at
+construction time by passing the relevant field (e.g.
+`database_stores.auth_key_store.clone()`) to each service constructor. Services
+themselves never see `DatabaseStores` — they receive only the narrow trait they
+need.
+
+The factory (`databases::driver::build` / `initialize_database`) constructs the
+concrete driver once and produces four `Arc<dyn XxxStore>` coercions from it:
+
+```rust
+pub struct DatabaseStores {
+    pub schema_migrator:       Arc<dyn SchemaMigrator>,
+    pub torrent_metrics_store: Arc<dyn TorrentMetricsStore>,
+    pub whitelist_store:       Arc<dyn WhitelistStore>,
+    pub auth_key_store:        Arc<dyn AuthKeyStore>,
+}
+
+pub fn initialize_database(config: &Core) -> DatabaseStores {
+    match config.database.driver {
+        Driver::Sqlite3 => {
+            let db = Arc::new(Sqlite::new(&config.database.path).expect("..."));
+            db.create_database_tables().expect("...");
+            DatabaseStores {
+                schema_migrator:       db.clone(),
+                torrent_metrics_store: db.clone(),
+                whitelist_store:       db.clone(),
+                auth_key_store:        db,
+            }
+        }
+        Driver::MySQL => { /* same pattern */ }
+    }
+}
+```
+
+## Tasks
+
+### 1) Introduce `DatabaseStores`
+
+Add a plain struct `databases::setup::DatabaseStores` holding one `Arc<dyn XxxStore>`
+per narrow trait. No `Arc<Box<dyn Database>>`.
+
+### 2) Update `initialize_database`
+
+Change the return type from `Arc<Box<dyn Database>>` to `DatabaseStores`.
+Build the concrete driver, call `create_database_tables`, then produce the four
+coercions.
+
+### 3) Update `TrackerCoreContainer`
+
+- Replace `pub database: Arc<Box<dyn Database>>` with `pub database_stores: DatabaseStores`.
+- Update `initialize_from` to call `initialize_database` (which now returns
+  `DatabaseStores`) and fan the narrow stores out to each service constructor:
+
+  ```rust
+  let db = initialize_database(core_config);
+  let whitelist_manager = initialize_whitelist_manager(db.whitelist_store.clone(), ...);
+  let db_key_repository = Arc::new(DatabaseKeyRepository::new(db.auth_key_store.clone()));
+  let db_downloads = Arc::new(DatabaseDownloadsMetricRepository::new(db.torrent_metrics_store.clone()));
+  // ... store the struct itself so callers can still access it if needed
+  Self { database_stores: db, ... }
+  ```
+
+### 4) Update individual consumers
+
+- `DatabaseKeyRepository::new` — accept `Arc<dyn AuthKeyStore>` instead of
+  `Arc<Box<dyn Database>>`.
+- `DatabaseDownloadsMetricRepository::new` — accept `Arc<dyn TorrentMetricsStore>`.
+- `whitelist::setup::initialize_whitelist_manager` — accept `Arc<dyn WhitelistStore>`.
+
+### 5) Update tests in `authentication/handler.rs`
+
+Replace `Arc<Box<dyn Database>>` wiring with `MockAuthKeyStore` injected
+directly as `Arc<dyn AuthKeyStore>`.
+
+### 6) Update `axum-rest-tracker-api-server` test helper
+
+`packages/axum-rest-tracker-api-server/tests/server/mod.rs::force_database_error`
+currently receives `&Arc<Box<dyn Database>>`. Update to the narrow trait(s) it
+actually exercises.
+
+### 7) Update benchmark binary
+
+`bin/persistence_benchmark/driver_bench/` passes `&dyn Database` to operations
+that each touch only one concern. Update each operation function to accept the
+narrow trait it needs:
+
+- `operations/torrent.rs` → `&dyn TorrentMetricsStore`
+- `operations/whitelist.rs` → `&dyn WhitelistStore`
+- `operations/keys.rs` → `&dyn AuthKeyStore`
+- `database/mod.rs::reset_database` → `&dyn SchemaMigrator`
+
+### 8) Make `Database` private
+
+Once no production or test code outside `databases/` uses `Database`, stop
+re-exporting it from `databases/mod.rs`. Keep it accessible inside
+`databases/traits/database.rs` for driver authors.
+
+## Out of Scope
+
+- Async trait methods. That is subissue #1525-05.
+- Schema migrations. That is subissue #1525-06.
+- PostgreSQL support. That is subissue #1525-08.
+
+## Acceptance Criteria
+
+- [ ] `Arc<Box<dyn Database>>` appears only inside `databases/` (driver + traits).
+- [ ] Each consumer holds only the narrow trait(s) it uses.
+- [ ] `Database` is no longer re-exported from `databases/mod.rs`.
+- [ ] Tests in `authentication/handler.rs` use `MockAuthKeyStore` directly.
+- [ ] `force_database_error` helper in `axum-rest-tracker-api-server` is updated.
+- [ ] Benchmark operations accept narrow traits.
+- [ ] `cargo test --workspace --all-targets` passes.
+- [ ] `linter all` exits with code `0`.
+
+## References
+
+- EPIC: #1525
+- Predecessor: [docs/issues/1713-1525-04-split-persistence-traits.md](1713-1525-04-split-persistence-traits.md)
+- ADR: [docs/adrs/20260429000000_keep_database_as_aggregate_supertrait.md](../adrs/20260429000000_keep_database_as_aggregate_supertrait.md)
+- Successor: [docs/issues/1525-05-migrate-sqlite-and-mysql-to-sqlx.md](1525-05-migrate-sqlite-and-mysql-to-sqlx.md)

docs/issues/1525-05-migrate-sqlite-and-mysql-to-sqlx.md

@@ -268,7 +268,7 @@ and all `r2d2`/`rusqlite`/`mysql` dependencies are gone.
 ## References
 
 - EPIC: `#1525`
-- Subissue `1525-04`: `docs/issues/1525-04-split-persistence-traits.md` — must be completed first
+- Subissue `1525-04`: `docs/issues/1713-1525-04-split-persistence-traits.md` — must be completed first
 - Subissue `1525-03`: `docs/issues/1525-03-persistence-benchmarking.md` — benchmark baseline
 - Reference PR: `#1695`
 - Reference implementation branch: `josecelano:pr-1684-review` — see EPIC for checkout

docs/issues/1525-overhaul-persistence.md

@@ -103,9 +103,15 @@ You can then browse or search it while working in the main repository.
 
 ### 4) Split the persistence traits by context
 
-- Spec file: `docs/issues/1525-04-split-persistence-traits.md`
+- Spec file: `docs/issues/1713-1525-04-split-persistence-traits.md`
 - Outcome: smaller interfaces with lower coupling and clearer responsibilities
 
+### 4b) Migrate consumers to narrow persistence traits
+
+- Spec file: `docs/issues/1525-04b-migrate-consumers-to-narrow-traits.md`
+- Outcome: every consumer holds only the narrow trait(s) it uses; `Database`
+  becomes a private compile-time guard inside `databases/`
+
 ### 5) Migrate SQLite and MySQL drivers to async `sqlx`
 
 - Spec file: `docs/issues/1525-05-migrate-sqlite-and-mysql-to-sqlx.md`

docs/issues/1525-04-split-persistence-traits.md → docs/issues/1713-1525-04-split-persistence-traits.md

@@ -1,4 +1,4 @@
-# Subissue Draft for #1525-04: Split Persistence Traits by Context
+# Issue #1713 (Subissue of #1525-04): Split Persistence Traits by Context
 
 ## Goal
 
@@ -43,7 +43,7 @@ This preserves both goals:
 
 ## Proposed Branch
 
-- `1525-04-split-persistence-traits`
+- `1713-1525-04-split-persistence-traits`
 
 ## Current State
 
@@ -246,6 +246,22 @@ pub use torrent_metrics::{MockTorrentMetricsStore, TorrentMetricsStore};
 pub use whitelist::{MockWhitelistStore, WhitelistStore};
 ```
 
+## Implementation Notes
+
+- **`mockall` dependency**: Already present in `[dependencies]` of `tracker-core/Cargo.toml`.
+  No change needed.
+
+- **ADR timestamp**: Use the date the ADR is authored (`YYYYMMDDHHMMSS` format, today's date).
+
+- **Consumer file changes**: The spirit of this subissue is not to mix refactorings — keep the
+  focus on the structural split. However, if test-only code (e.g. `MockDatabase` usage in
+  `handler.rs`) must be updated to compile after `MockDatabase` is removed, that change is
+  acceptable. Production consumer files (`persisted.rs`, `downloads.rs`, etc.) must not change.
+
+- **Method signatures**: Follow the actual code in `mod.rs` — the spec snippets are suggestions
+  and may have drifted. In particular, `save_torrent_downloads` takes `completed: u32` (not
+  `NumberOfDownloads`) in the current code.
+
 ## Out of Scope
 
 - Changing consumer wiring from `Arc<Box<dyn Database>>` to narrow trait objects.
@@ -261,7 +277,8 @@ pub use whitelist::{MockWhitelistStore, WhitelistStore};
 - [ ] `Database` is an empty aggregate supertrait with a blanket impl.
 - [ ] Both drivers (`Sqlite`, `Mysql`) compile through the blanket impl with no manual
       `impl Database for <Driver>` block.
-- [ ] No existing consumer file (`persisted.rs`, `downloads.rs`, etc.) is changed.
+- [ ] Production consumer files (`persisted.rs`, `downloads.rs`, etc.) are not changed.
+- [ ] Test code that used `MockDatabase` is updated to use the appropriate narrow mock type.
 - [ ] `#[automock]` is on the four narrow traits; `MockDatabase` is removed.
 - [ ] No behavior change — existing tests pass without modification.
 - [ ] Persistence benchmarking (see subissue #1525-03) shows no regression against the