From 33cc41bacd3124fac47b87a33ee89e0ae974db71 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 14 Jul 2026 19:12:52 +0200 Subject: [PATCH] fix: make the genre dedup migration collision-proof (0038) Review caught a boot-crash risk: 0038's repoint-UPDATE could set two non-canonical variant links of one manga to the same canonical id in a single statement (NOT EXISTS sees the pre-statement snapshot) -> manga_genres PK violation -> migration rollback -> startup failure. Replace with INSERT ... SELECT DISTINCT ... ON CONFLICT DO NOTHING (collision-proof) then delete variants. Amended in place (0038 is unpushed). Tested with the exact collision scenario. Co-Authored-By: Claude Opus 4.8 --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- .../0038_genres_name_lower_unique.sql | 47 +++++----- backend/tests/crawler_sync.rs | 87 +++++++++++++++++++ frontend/package.json | 2 +- 5 files changed, 113 insertions(+), 27 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 8bc7ff0..f42be21 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.128.24" +version = "0.128.25" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index f509be6..fd38858 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.128.24" +version = "0.128.25" edition = "2021" default-run = "mangalord" diff --git a/backend/migrations/0038_genres_name_lower_unique.sql b/backend/migrations/0038_genres_name_lower_unique.sql index b9096f6..1fdde60 100644 --- a/backend/migrations/0038_genres_name_lower_unique.sql +++ b/backend/migrations/0038_genres_name_lower_unique.sql @@ -9,32 +9,31 @@ -- Heal any pre-existing case-variant duplicates before adding the index, so -- `sqlx::migrate!` can't crash on a dirty table (mirrors 0031's pre-dedup). --- Keep the lowest id per lower(name) as canonical and repoint manga_genres. -UPDATE manga_genres mg - SET genre_id = d.keep_id - FROM ( - SELECT g.id AS dup_id, k.keep_id - FROM genres g - JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id - FROM genres GROUP BY lower(name)) k - ON lower(g.name) = k.lname AND g.id <> k.keep_id - ) d - WHERE mg.genre_id = d.dup_id - AND NOT EXISTS ( - SELECT 1 FROM manga_genres x - WHERE x.manga_id = mg.manga_id AND x.genre_id = d.keep_id - ); +-- Canonical = lowest id per lower(name). --- Drop links that would have collided with the canonical link (already present). +-- 1. Ensure every manga linked to any case-variant also has the canonical link. +-- INSERT ... ON CONFLICT DO NOTHING is collision-proof: SELECT DISTINCT +-- collapses multiple variants of one manga to a single canonical row, and the +-- ON CONFLICT absorbs a canonical link that already exists. (A prior +-- UPDATE-repoint could set two non-canonical rows of the SAME manga to the +-- same canonical id in a single statement and violate the manga_genres PK, +-- rolling the migration back and wedging startup.) +INSERT INTO manga_genres (manga_id, genre_id) +SELECT DISTINCT mg.manga_id, k.keep_id + FROM manga_genres mg + JOIN genres g ON mg.genre_id = g.id + JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id + FROM genres GROUP BY lower(name)) k + ON lower(g.name) = k.lname + WHERE g.id <> k.keep_id +ON CONFLICT (manga_id, genre_id) DO NOTHING; + +-- 2. Every non-canonical link now has a canonical sibling — drop the variants. DELETE FROM manga_genres mg - USING ( - SELECT g.id AS dup_id - FROM genres g - JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id - FROM genres GROUP BY lower(name)) k - ON lower(g.name) = k.lname AND g.id <> k.keep_id - ) d - WHERE mg.genre_id = d.dup_id; + USING genres g, + (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id + FROM genres GROUP BY lower(name)) k + WHERE mg.genre_id = g.id AND lower(g.name) = k.lname AND g.id <> k.keep_id; -- Remove the now-orphaned duplicate genre rows. DELETE FROM genres g diff --git a/backend/tests/crawler_sync.rs b/backend/tests/crawler_sync.rs index 5c4ebc2..c4af567 100644 --- a/backend/tests/crawler_sync.rs +++ b/backend/tests/crawler_sync.rs @@ -674,6 +674,93 @@ async fn arbitrary_genres_from_source_get_inserted(pool: PgPool) { assert_eq!(webtoons_count.0, 1, "case-insensitive lookup reuses the existing row"); } +#[sqlx::test(migrations = "./migrations")] +async fn genre_dedup_survives_a_manga_linked_to_two_variants(pool: PgPool) { + // Regression for a migration-0038 collision: a manga linked to TWO + // non-canonical case-variants of one genre. A repoint-UPDATE would set both + // rows to the canonical id in one statement -> manga_genres PK violation -> + // migration rollback -> boot failure. #[sqlx::test] migrates a clean DB, so + // recreate the dirty pre-index state and replay 0038's healing statements; + // they must complete and leave exactly the one canonical link. (Mirrors the + // healing SQL in migrations/0038_genres_name_lower_unique.sql.) + let manga = Uuid::new_v4(); + sqlx::query("INSERT INTO mangas (id, title) VALUES ($1, 'T')") + .bind(manga) + .execute(&pool) + .await + .unwrap(); + + // Drop the live guard so we can plant case-variant genres. + sqlx::query("DROP INDEX genres_name_lower_uniq").execute(&pool).await.unwrap(); + let keep = Uuid::parse_str("00000000-0000-0000-0000-000000000001").unwrap(); + let dup2 = Uuid::parse_str("00000000-0000-0000-0000-000000000002").unwrap(); + let dup3 = Uuid::parse_str("00000000-0000-0000-0000-000000000003").unwrap(); + for (id, name) in [(keep, "Zzz"), (dup2, "zzz"), (dup3, "ZZZ")] { + sqlx::query("INSERT INTO genres (id, name) VALUES ($1, $2)") + .bind(id) + .bind(name) + .execute(&pool) + .await + .unwrap(); + } + // The manga links to the two NON-canonical variants, not the canonical one. + for gid in [dup2, dup3] { + sqlx::query("INSERT INTO manga_genres (manga_id, genre_id) VALUES ($1, $2)") + .bind(manga) + .bind(gid) + .execute(&pool) + .await + .unwrap(); + } + + // Step 1 — collision-proof canonical-link backfill (the crux of the fix). + sqlx::query( + "INSERT INTO manga_genres (manga_id, genre_id) \ + SELECT DISTINCT mg.manga_id, k.keep_id \ + FROM manga_genres mg JOIN genres g ON mg.genre_id = g.id \ + JOIN (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \ + FROM genres GROUP BY lower(name)) k ON lower(g.name) = k.lname \ + WHERE g.id <> k.keep_id \ + ON CONFLICT (manga_id, genre_id) DO NOTHING", + ) + .execute(&pool) + .await + .expect("collision-proof INSERT must not raise a manga_genres PK violation"); + // Step 2 — drop the non-canonical links. + sqlx::query( + "DELETE FROM manga_genres mg USING genres g, \ + (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \ + FROM genres GROUP BY lower(name)) k \ + WHERE mg.genre_id = g.id AND lower(g.name) = k.lname AND g.id <> k.keep_id", + ) + .execute(&pool) + .await + .unwrap(); + // Step 3 — remove orphaned duplicate genres. + sqlx::query( + "DELETE FROM genres g USING \ + (SELECT lower(name) AS lname, (array_agg(id ORDER BY id))[1] AS keep_id \ + FROM genres GROUP BY lower(name)) k \ + WHERE lower(g.name) = k.lname AND g.id <> k.keep_id", + ) + .execute(&pool) + .await + .unwrap(); + + // Exactly the canonical link remains, and the guard re-creates cleanly. + let links: Vec = + sqlx::query_scalar("SELECT genre_id FROM manga_genres WHERE manga_id = $1") + .bind(manga) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!(links, vec![keep], "manga keeps exactly the canonical genre link"); + sqlx::query("CREATE UNIQUE INDEX genres_name_lower_uniq ON genres (lower(name))") + .execute(&pool) + .await + .expect("no residual case-variant duplicates remain"); +} + #[sqlx::test(migrations = "./migrations")] async fn genres_reject_case_variant_duplicates_at_the_db(pool: PgPool) { // The sequential pre-check in sync_genres dedups the common case, but two diff --git a/frontend/package.json b/frontend/package.json index feb7e47..9ed6bfd 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.128.24", + "version": "0.128.25", "private": true, "type": "module", "scripts": {