feat(tracker-core): introduce sqlx schema migrations (subissue 1525-06)#1720
Merged
josecelano merged 8 commits intotorrust:developfrom Apr 30, 2026
Merged
Conversation
…ust#1719 Renames docs/issues/1525-06-introduce-schema-migrations.md to docs/issues/1719-1525-06-introduce-schema-migrations.md so the filename includes the GitHub issue number, matching the sibling-subissue naming pattern (<issue-number>-<spec-slug>.md) already used by 1717-1525-05-migrate-sqlite-and-mysql-to-sqlx.md. Updates the three internal references to the renamed file in: - docs/issues/1525-overhaul-persistence.md - docs/issues/1525-07-align-rust-and-db-types.md (two references) - docs/issues/1525-08-add-postgresql-driver.md Documentation only; no code changes.
Add 11 findings (F1–F11) before the Tasks section reflecting the actual develop-branch state after subissue 1525-05, and rewrite spec passages and code blocks to match. Highlights: - Remove obsolete references to an ensure_schema() latch (1525-05 explicitly chose not to use one). - Correct the false claim that drop_database_tables() omits torrent_aggregate_metrics; only _sqlx_migrations is the new drop. - Align the proposed error API with the existing tuple-From pattern: introduce an Error::MigrationError variant and impl From<(MigrateError, Driver)> for Error instead of a free-standing Error::migration_error() constructor. - Add 'notnull' to the cspell dictionary for the SQLite PRAGMA table_info reference.
…ases
Capture the implementer Q&A (Q1-Q7 plus follow-up Q1.5) inline in the
spec and rework the plan based on the answers:
- Reorganize Tasks into three commit-sized phases:
1. Scaffolding (Tasks 1+2): add sqlx-migrate, embed migrations,
and apply the SQLite-only # -> -- comment fix in
migration 1. MySQL migration 1 is left untouched (Q1.5).
2. Fresh-install path (Task 3): wire migrations into bootstrap for
empty databases.
3. Legacy bootstrap (Task 4): detect pre-v4 schemas and baseline
them; precondition simplified to all four legacy tables
present with no column-level checks (Q4).
- Rewrite Acceptance Criteria to match the new split: SQLite migration
1 is the only file edit, both backends are exercised for
fresh/idempotency/legacy-bootstrap/partial-migration (MySQL gated),
and _sqlx_migrations is the only newly required drop (F2).
- Document upgrade-from-older-versions in the README only, instead of
publishing a separate v4 changelog entry (Q5).
- Add Out-of-Scope items: defer INT(10) -> INT normalization
to subissue 1525-07 (Q2) and defer the metrics ->
torrent_metrics rename (Q1.5).
- Patch stale Findings: F5 no longer claims the MySQL migration must
be edited, and F3 now points at Task 4 instead of Task 3.
- Minor: normalize behavioural -> behavioral to satisfy
cspell.
Lays the groundwork for managing schema with sqlx migrations as part of subissue 1525-06 (issue torrust#1719). No migrations are executed yet; wiring into table creation will follow in the next phase. Changes: - Enable the sqlx `macros` feature so `sqlx::migrate!` can embed the per-driver migration directories at compile time. - Embed per-driver `MIGRATOR` statics in the SQLite and MySQL driver modules, pointing at `migrations/sqlite` and `migrations/mysql` respectively. They are marked `#[allow(dead_code)]` until invoked. - Add an `Error::MigrationError` variant and a `From<(MigrateError, Driver)>` impl, mirroring the existing tuple-From pattern used for `SqlxError` so call sites can use `?` uniformly. - Replace the unsupported `#` comment with `--` on the first SQLite migration so the embedded migrator can parse it. The MySQL migration is intentionally left untouched: MySQL accepts `#` comments, and editing a published migration would break installer immutability. Refs: subissue 1525-06 (torrust#1719)
Replace the hand-written CREATE TABLE statements in both database drivers with the embedded `sqlx` migrator so schema bootstrap and future schema evolution share a single, version-tracked code path. - `create_database_tables()` on SQLite and MySQL now runs `MIGRATOR.run(&self.pool)`, applying every embedded migration idempotently and recording state in the `_sqlx_migrations` table. - `drop_database_tables()` switches to `DROP TABLE IF EXISTS` and also drops `_sqlx_migrations`, so test teardown leaves a truly empty schema and the next `create_database_tables()` call re-applies every migration from a clean state. - Removes the now-stale `AUTH_KEY_LENGTH` coupling from the MySQL driver; the `keys.key` column width lives in the migration file instead. - Rewrites `packages/tracker-core/migrations/README.md` to document the new automatic-migrations behavior, the `_sqlx_migrations` tracking table, how to add new migrations, the immutability rule for applied migrations, and the pre-v4 upgrade path. - Adds an idempotency test on SQLite (`create_database_tables_should_be_idempotent_on_a_fresh_database`, always on) and an idempotency assertion in the gated MySQL driver test (`TORRUST_TRACKER_CORE_RUN_MYSQL_DRIVER_TEST=true`). Fresh installs now bootstrap end-to-end through the migrator. Pre-v4 installs are still expected to fail at startup because their schemas predate `_sqlx_migrations`; that legacy-bootstrap path is addressed in the next task of this subissue. Refs subissue 1525-06 (torrust#1719) — fresh-install phase.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1720 +/- ##
===========================================
- Coverage 81.16% 80.89% -0.27%
===========================================
Files 348 348
Lines 24917 25162 +245
Branches 24917 25162 +245
===========================================
+ Hits 20225 20356 +131
- Misses 4471 4579 +108
- Partials 221 227 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds per-driver `bootstrap_legacy_schema` free function (SQLite + MySQL) that detects pre-v4 user-managed schemas and fake-applies the three legacy migrations into `_sqlx_migrations` using the embedded checksums, so subsequent `MIGRATOR.run()` is a no-op. Adds `Error::LegacyDatabaseNotMigrated` to reject partial pre-v4 states (some legacy tables missing) with an actionable message. Wired into `SchemaMigrator::create_database_tables` for both drivers; runs once before the embedded migrator. SQLite: three unit tests covering fresh-noop, fully-migrated bootstrap, and partial-state rejection. Extracted `create_legacy_pre_v4_schema` helper for the legacy DDL. MySQL: extends `run_mysql_driver_tests` (gated by `db-compatibility-tests` feature + `TORRUST_TRACKER_CORE_RUN_MYSQL_DRIVER_TEST=true`) with legacy bootstrap + partial-state assertions using the same helper pattern. Added rust-doc on the legacy constants, free functions and test helpers documenting that they form a single removable compatibility layer, with a 4-step recipe for deleting it when pre-v4 support is dropped. Refs torrust#1719 (subissue 1525-06).
There was a problem hiding this comment.
Pull request overview
Introduces sqlx::migrate!()-based embedded schema migrations for bittorrent-tracker-core’s SQLite and MySQL async drivers, replacing hardcoded DDL and adding a legacy bootstrap path for pre-v4 databases without _sqlx_migrations.
Changes:
- Embed and run per-backend
sqlxmigrators on startup; switch drop paths toDROP TABLE IF EXISTSincluding_sqlx_migrations. - Add migration-focused error variants and conversions (
MigrationError, plus a dedicated legacy-precondition error). - Update migration documentation/specs and add idempotency + legacy-bootstrap tests (SQLite always-on; MySQL in gated compatibility tests).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| project-words.txt | Adds notnull to project dictionary. |
| packages/tracker-core/Cargo.toml | Enables sqlx macros to support sqlx::migrate!(). |
| packages/tracker-core/src/databases/error.rs | Adds migration/legacy-bootstrap error variants and From<(MigrateError, Driver)>. |
| packages/tracker-core/src/databases/driver/sqlite/mod.rs | Defines embedded SQLite MIGRATOR and adds idempotency test. |
| packages/tracker-core/src/databases/driver/sqlite/schema_migrator.rs | Replaces raw DDL with migrator + legacy bootstrap; updates drop logic; adds legacy-bootstrap tests. |
| packages/tracker-core/src/databases/driver/mysql/mod.rs | Defines embedded MySQL MIGRATOR and extends gated MySQL tests for idempotency/legacy bootstrap. |
| packages/tracker-core/src/databases/driver/mysql/schema_migrator.rs | Replaces raw DDL with migrator + legacy bootstrap; updates drop logic. |
| packages/tracker-core/migrations/sqlite/20240730183000_torrust_tracker_create_all_tables.sql | Fixes SQLite-incompatible # comment to -- so embedded migrator can parse it. |
| packages/tracker-core/migrations/README.md | Updates migration docs for automatic embedded migrations and legacy upgrade expectations. |
| docs/issues/1719-1525-06-introduce-schema-migrations.md | Adds the detailed spec document for subissue 1525-06 (as issue #1719). |
| docs/issues/1525-overhaul-persistence.md | Updates spec reference to the new 1719-prefixed filename. |
| docs/issues/1525-08-add-postgresql-driver.md | Updates spec reference to the new 1719-prefixed filename. |
| docs/issues/1525-07-align-rust-and-db-types.md | Updates link to history-alignment section in the renamed spec doc. |
| docs/issues/1525-06-introduce-schema-migrations.md | Removes the older spec file in favor of the new renamed document. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add the legacy-bootstrap timestamp cutoff (`> 20250527093000`) so contributors don't accidentally write a new migration that gets fake-applied on legacy databases. - Note that `sqlx-cli` is an optional filename generator; manual creation is equally valid. - State the forward-only convention (no `.up.sql`/`.down.sql` pairs). - Generalise the comment-syntax note across both backends. - Mention that a rebuild is required for new migrations to be embedded. Refs torrust#1719 (subissue 1525-06).
- Fix stale "(both backends)" wording in the subissue 1525-06 spec — Task 1 only edits the SQLite migration; MySQL migration 1 must remain untouched per the immutability rule. - Build the SQLite test pool through `SqliteConnectOptions::filename` (mirroring the production driver) so non-UTF-8 and Windows paths are handled correctly; drop the brittle `to_str().unwrap()` URL-formatting in the test helper. Refs torrust#1719 (subissue 1525-06), PR torrust#1720.
Member
Author
|
ACK d6c2f65 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements subissue 1525-06 of the persistence overhaul (#1525): adopt automatic schema migrations using
sqlx::migrate!.Spec: docs/issues/1719-1525-06-introduce-schema-migrations.md
Closes #1719.
Tasks
#comment in migration 1 so the embedded migrator can parse it.macrosfeature, add per-driverMIGRATORstatics, addError::MigrationError+Fromimpl.MIGRATOR.run()intocreate_database_tables(), switchdrop_database_tables()toDROP TABLE IF EXISTS(incl._sqlx_migrations), rewritemigrations/README.md, add idempotency tests._sqlx_migrationsand seed the migration history so the embedded migrator skips them on subsequent runs. Tests on both backends.Both fresh installs and pre-v4 in-place upgrades work end-to-end. The legacy-bootstrap layer is documented as a single removable unit (see the rust-doc on
bootstrap_legacy_schemain each driver) so it can be dropped cleanly when pre-v4 support is no longer required.