From f30600162e8e6ed2fbc4aed8eb09187740b77f34 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 13 Jun 2026 17:44:38 +0200 Subject: [PATCH] feat(page-tags): normalize page_tags onto the shared tags table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit page_tags previously stored the tag inline as free-form text — the lone un-normalized tag concept, while authors/genres/manga-tags all went through a lookup table + FK. Fold page tags into the SAME shared `tags` table that manga_tags uses: `page_tags.tag` becomes `page_tags.tag_id` referencing `tags(id)`. Manga tags and page tags now share one global vocabulary. The HTTP contract is unchanged — the API still speaks tag *names*; the repo resolves name<->id via `repo::tag::upsert_by_name` (the manga-tag helper) and keeps the stricter page-tag `normalize_tag` at the boundary. Frontend, DTOs, and response shapes are untouched. User-visible effect: page-tag names now appear in the manga-tag autocomplete and vice versa. Migration 0024 backfills `tags` from existing inline values (deduping case-insensitively), repoints rows, swaps the unique/secondary indexes to tag_id, and drops the inline column. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 2 +- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- .../migrations/0024_normalize_page_tags.sql | 48 +++++++++ backend/src/repo/page_tag.rs | 101 +++++++++++------- backend/tests/api_page_tags.rs | 66 ++++++++++++ frontend/package.json | 2 +- 7 files changed, 182 insertions(+), 41 deletions(-) create mode 100644 backend/migrations/0024_normalize_page_tags.sql diff --git a/CLAUDE.md b/CLAUDE.md index 613a913..2bd152c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,7 +136,7 @@ docker compose -f docker-compose.dev.yml up -d These are first-class slots in the architecture. When adding any of them, plug into the existing seam rather than building parallel infrastructure. - **Tags / lists**: new tables joined to `mangas`. New `domain`, `repo`, and `api` modules; the existing manga endpoints do not need to change. -- **Per-page collections / tags**: `collections` is heterogeneous — `collection_mangas` holds whole mangas, `collection_pages` holds individual pages (FK to `pages.id`). Per-user personal page tags live in `page_tags` (distinct from the shared manga `tags` taxonomy; free-form text, normalized at the API layer). Both tables cascade-delete with `pages` and `chapters`, so re-uploading a chapter drops saved-page references by design. +- **Per-page collections / tags**: `collections` is heterogeneous — `collection_mangas` holds whole mangas, `collection_pages` holds individual pages (FK to `pages.id`). Per-user page tags live in `page_tags`, which references the **shared** `tags` table by `tag_id` (migration 0024) — the same lookup table `manga_tags` uses, so manga tags and page tags share one global vocabulary. The HTTP contract still speaks tag *names*; `repo::page_tag` resolves name↔id via `repo::tag::upsert_by_name` and applies the stricter page-tag `normalize_tag` (lowercase, collapse whitespace, reject wildcards/control/invisible chars) at the API layer. Both `collection_pages` and `page_tags` cascade-delete with `pages` and `chapters`, so re-uploading a chapter drops saved-page references by design. - **Tag-based content search (`/search`)**: the user-facing search surface lives at [frontend/src/routes/search/+page.svelte](frontend/src/routes/search/+page.svelte). Three result views (Pages / Chapters / Mangas) consume the matching `/v1/me/page-tags`, `/v1/me/page-tags/chapters`, and `/v1/me/page-tags/mangas` endpoints. Note the two distinct query-param spaces: `?q=` on `/v1/me/page-tags` is a tag-name prefix (for autocomplete in the "Add tag" sheet); `?text=` on the aggregation endpoints is **reserved** for the planned OCR text-search input. Both aggregation handlers accept `text=` on the wire but reject non-empty values with 501 `text_search_not_yet_supported` so adding OCR later doesn't break the API shape. Adding OCR is then: a background worker writes `page_ocr_text` rows, a JOIN on the existing aggregation queries adds the new filter, the `text=` param starts validating instead of rejecting. - **Full-text / fuzzy search**: enable `pg_trgm` in a migration and add a GIN index on `mangas.title`; swap the `WHERE` in `repo::manga::list` to use `%` operator or `tsvector`. The API shape (`?search=...`) does not change. - **OCR / autotagging**: a background worker (a separate binary or a tokio task spawned in `app::build`) that reads pages from `storage::Storage` and writes tag rows. Do not couple OCR to upload handlers — it runs asynchronously. diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 54a8687..96d5878 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1470,7 +1470,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.63.0" +version = "0.64.0" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index b29143e..ba2a52d 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.63.0" +version = "0.64.0" edition = "2021" default-run = "mangalord" diff --git a/backend/migrations/0024_normalize_page_tags.sql b/backend/migrations/0024_normalize_page_tags.sql new file mode 100644 index 0000000..2fe0270 --- /dev/null +++ b/backend/migrations/0024_normalize_page_tags.sql @@ -0,0 +1,48 @@ +-- Normalize per-page tags. Originally (0023) `page_tags.tag` stored the +-- tag inline as free-form text, the lone un-normalized tag concept while +-- authors/genres/manga-tags all went through a lookup table + FK. This +-- migration folds page tags into the SAME shared `tags` table that +-- `manga_tags` uses (0009): `page_tags.tag` becomes `page_tags.tag_id` +-- referencing `tags(id)`. After this there is one global tag vocabulary +-- shared by manga tags and page tags. +-- +-- The HTTP contract is unchanged — the API still speaks tag *names*; the +-- repo resolves name<->id via `repo::tag::upsert_by_name`, exactly as the +-- manga-tag path does. + +-- 1. New FK column, nullable until backfilled. +ALTER TABLE page_tags + ADD COLUMN tag_id uuid REFERENCES tags(id) ON DELETE CASCADE; + +-- 2. Seed the lookup table from the existing inline values, deduping +-- case-insensitively via the tags (lower(name)) unique index. Mirrors +-- the author backfill in 0009. +INSERT INTO tags (name) + SELECT DISTINCT tag FROM page_tags +ON CONFLICT (lower(name)) DO NOTHING; + +-- 3. Point every page_tags row at its canonical tag row. +UPDATE page_tags pt + SET tag_id = t.id + FROM tags t + WHERE lower(t.name) = lower(pt.tag); + +-- 4. The FK is now mandatory. +ALTER TABLE page_tags ALTER COLUMN tag_id SET NOT NULL; + +-- 5. Swap the inline-tag indexes/constraints for tag_id equivalents. +DROP INDEX IF EXISTS page_tags_user_page_tag_uniq; +CREATE UNIQUE INDEX page_tags_user_page_tag_uniq + ON page_tags (user_id, page_id, tag_id); + +DROP INDEX IF EXISTS page_tags_user_tag_idx; +CREATE INDEX page_tags_user_tag_idx + ON page_tags (user_id, tag_id, created_at DESC); + +-- page_tags_user_idx (user_id, created_at DESC) and page_tags_page_idx +-- (page_id) are unaffected and stay as-is. + +-- 6. Drop the now-dead inline column. This also drops the +-- page_tags_tag_nonempty CHECK; the 1..=64 length bound is now +-- enforced by `repo::tag::upsert_by_name` before a row is created. +ALTER TABLE page_tags DROP COLUMN tag; diff --git a/backend/src/repo/page_tag.rs b/backend/src/repo/page_tag.rs index 2a96994..755fd47 100644 --- a/backend/src/repo/page_tag.rs +++ b/backend/src/repo/page_tag.rs @@ -1,10 +1,14 @@ //! Per-user, per-page tag persistence. //! -//! Same plain-function pattern as the rest of `repo`. Idempotent -//! upserts via `ON CONFLICT DO NOTHING` (the caller distinguishes -//! 201/200 from the returned `bool`), FK-violation remap to NotFound so -//! the handler can return 404 when the page was deleted between an -//! existence check and the insert. +//! Same plain-function pattern as the rest of `repo`. Tag names resolve +//! to the shared `tags` table (the same one `manga_tags` uses) via +//! [`crate::repo::tag::upsert_by_name`]; `page_tags` links rows by +//! `tag_id` (see migration 0024). The API still speaks tag *names* — the +//! name<->id resolution lives entirely here. Idempotent upserts via +//! `ON CONFLICT DO NOTHING` (the caller distinguishes 201/200 from the +//! returned `bool`), FK-violation remap to NotFound so the handler can +//! return 404 when the page was deleted between an existence check and +//! the insert. use sqlx::PgPool; use uuid::Uuid; @@ -33,26 +37,30 @@ impl Order { } } -/// Insert a normalized tag for `(user_id, page_id)`. Returns `true` if -/// a new row was inserted (handler → 201), `false` if the tag was -/// already present (handler → 200). +/// Insert a tag for `(user_id, page_id)`. The tag name is resolved to +/// (or created in) the shared `tags` table first — the same table and +/// `upsert_by_name` helper the manga-tag path uses — then linked by +/// `tag_id`. Returns `true` if a new link was inserted (handler → 201), +/// `false` if the tag was already present (handler → 200). pub async fn upsert( pool: &PgPool, user_id: Uuid, page_id: Uuid, tag: &str, ) -> AppResult { + let mut tx = pool.begin().await?; + let tag_row = crate::repo::tag::upsert_by_name(&mut *tx, tag).await?; let result = sqlx::query( r#" - INSERT INTO page_tags (user_id, page_id, tag) + INSERT INTO page_tags (user_id, page_id, tag_id) VALUES ($1, $2, $3) - ON CONFLICT (user_id, page_id, tag) DO NOTHING + ON CONFLICT (user_id, page_id, tag_id) DO NOTHING "#, ) .bind(user_id) .bind(page_id) - .bind(tag) - .execute(pool) + .bind(tag_row.id) + .execute(&mut *tx) .await .map_err(|e| match e { sqlx::Error::Database(ref db_err) if db_err.is_foreign_key_violation() => { @@ -60,9 +68,13 @@ pub async fn upsert( } other => AppError::Database(other), })?; + tx.commit().await?; Ok(result.rows_affected() > 0) } +/// Remove a tag from `(user_id, page_id)`. The tag arrives as a name; we +/// resolve it to the shared `tags` row case-insensitively in a subquery +/// so a no-op delete (unknown name) simply matches nothing. pub async fn remove( pool: &PgPool, user_id: Uuid, @@ -70,7 +82,12 @@ pub async fn remove( tag: &str, ) -> AppResult<()> { sqlx::query( - "DELETE FROM page_tags WHERE user_id = $1 AND page_id = $2 AND tag = $3", + r#" + DELETE FROM page_tags + WHERE user_id = $1 + AND page_id = $2 + AND tag_id = (SELECT id FROM tags WHERE lower(name) = lower($3)) + "#, ) .bind(user_id) .bind(page_id) @@ -89,10 +106,11 @@ pub async fn list_for_page( ) -> AppResult> { let rows: Vec<(String,)> = sqlx::query_as( r#" - SELECT tag - FROM page_tags - WHERE user_id = $1 AND page_id = $2 - ORDER BY created_at ASC, tag + SELECT t.name AS tag + FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id + WHERE pt.user_id = $1 AND pt.page_id = $2 + ORDER BY pt.created_at ASC, t.name "#, ) .bind(user_id) @@ -143,7 +161,7 @@ pub async fn list_for_user( let rows = sqlx::query_as::<_, TaggedPageItem>( r#" SELECT - pt.tag AS tag, + t.name AS tag, p.id AS page_id, p.chapter_id AS chapter_id, ch.manga_id AS manga_id, @@ -154,12 +172,13 @@ pub async fn list_for_user( p.storage_key AS storage_key, pt.created_at AS tagged_at FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id JOIN pages p ON p.id = pt.page_id JOIN chapters ch ON ch.id = p.chapter_id JOIN mangas m ON m.id = ch.manga_id WHERE pt.user_id = $1 - AND ($2::text IS NULL OR pt.tag = $2) - AND ($3::text IS NULL OR pt.tag LIKE $3 || '%' ESCAPE '\') + AND ($2::text IS NULL OR lower(t.name) = $2) + AND ($3::text IS NULL OR lower(t.name) LIKE $3 || '%' ESCAPE '\') ORDER BY pt.created_at DESC, pt.id LIMIT $4 OFFSET $5 "#, @@ -176,10 +195,11 @@ pub async fn list_for_user( let (total,): (i64,) = sqlx::query_as( r#" SELECT count(*) - FROM page_tags - WHERE user_id = $1 - AND ($2::text IS NULL OR tag = $2) - AND ($3::text IS NULL OR tag LIKE $3 || '%' ESCAPE '\') + FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id + WHERE pt.user_id = $1 + AND ($2::text IS NULL OR lower(t.name) = $2) + AND ($3::text IS NULL OR lower(t.name) LIKE $3 || '%' ESCAPE '\') "#, ) .bind(user_id) @@ -204,12 +224,13 @@ pub async fn distinct_tags_for_user( let escaped_prefix = prefix.map(escape_like_prefix); let rows = sqlx::query_as::<_, PageTagSummary>( r#" - SELECT tag, count(*) AS count - FROM page_tags - WHERE user_id = $1 - AND ($2::text IS NULL OR tag LIKE $2 || '%' ESCAPE '\') - GROUP BY tag - ORDER BY count DESC, tag + SELECT t.name AS tag, count(*) AS count + FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id + WHERE pt.user_id = $1 + AND ($2::text IS NULL OR lower(t.name) LIKE $2 || '%' ESCAPE '\') + GROUP BY t.name + ORDER BY count DESC, t.name LIMIT $3 "#, ) @@ -252,9 +273,10 @@ pub async fn aggregate_chapters_for_tag( SELECT p.storage_key, p.page_number FROM pages p JOIN page_tags pt2 ON pt2.page_id = p.id + JOIN tags t2 ON t2.id = pt2.tag_id WHERE p.chapter_id = ch.id AND pt2.user_id = $1 - AND pt2.tag = $2 + AND lower(t2.name) = $2 ORDER BY p.page_number ASC LIMIT 3 ) p2 @@ -262,11 +284,12 @@ pub async fn aggregate_chapters_for_tag( ARRAY[]::text[] ) AS sample_storage_keys FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id JOIN pages p ON p.id = pt.page_id JOIN chapters ch ON ch.id = p.chapter_id JOIN mangas m ON m.id = ch.manga_id WHERE pt.user_id = $1 - AND pt.tag = $2 + AND lower(t.name) = $2 GROUP BY ch.id, ch.manga_id, m.title, ch.number, ch.title ORDER BY match_count {dir}, ch.id LIMIT $3 OFFSET $4 @@ -286,8 +309,9 @@ pub async fn aggregate_chapters_for_tag( SELECT count(*) FROM ( SELECT 1 FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id JOIN pages p ON p.id = pt.page_id - WHERE pt.user_id = $1 AND pt.tag = $2 + WHERE pt.user_id = $1 AND lower(t.name) = $2 GROUP BY p.chapter_id ) c "#, @@ -324,9 +348,10 @@ pub async fn aggregate_mangas_for_tag( FROM pages p JOIN chapters ch2 ON ch2.id = p.chapter_id JOIN page_tags pt2 ON pt2.page_id = p.id + JOIN tags t2 ON t2.id = pt2.tag_id WHERE ch2.manga_id = m.id AND pt2.user_id = $1 - AND pt2.tag = $2 + AND lower(t2.name) = $2 ORDER BY p.page_number ASC LIMIT 3 ) p2 @@ -334,11 +359,12 @@ pub async fn aggregate_mangas_for_tag( ARRAY[]::text[] ) AS sample_storage_keys FROM page_tags pt + JOIN tags t ON t.id = pt.tag_id JOIN pages p ON p.id = pt.page_id JOIN chapters ch ON ch.id = p.chapter_id JOIN mangas m ON m.id = ch.manga_id WHERE pt.user_id = $1 - AND pt.tag = $2 + AND lower(t.name) = $2 GROUP BY m.id, m.title, m.cover_image_path ORDER BY match_count {dir}, m.id LIMIT $3 OFFSET $4 @@ -358,9 +384,10 @@ pub async fn aggregate_mangas_for_tag( SELECT count(*) FROM ( SELECT 1 FROM page_tags pt - JOIN pages p ON p.id = pt.page_id + JOIN tags t ON t.id = pt.tag_id + JOIN pages p ON p.id = pt.page_id JOIN chapters ch ON ch.id = p.chapter_id - WHERE pt.user_id = $1 AND pt.tag = $2 + WHERE pt.user_id = $1 AND lower(t.name) = $2 GROUP BY ch.manga_id ) m "#, diff --git a/backend/tests/api_page_tags.rs b/backend/tests/api_page_tags.rs index e648678..c855254 100644 --- a/backend/tests/api_page_tags.rs +++ b/backend/tests/api_page_tags.rs @@ -418,6 +418,72 @@ async fn tags_are_per_user_only(pool: PgPool) { assert_eq!(body["tags"], json!([])); } +#[sqlx::test(migrations = "./migrations")] +async fn page_tag_and_manga_tag_share_one_tags_row(pool: PgPool) { + // The point of the normalization refactor: page tags now reference + // the shared `tags` table. A manga tag "Funny" and a page tag + // "funny" (case-folded) must resolve to a single `tags` row, and + // `page_tags.tag_id` must point at the same row as `manga_tags`. + let db = pool.clone(); + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + let manga_id = common::seed_manga_via_api(&h.app, &cookie, "M").await; + let (_, page_id) = seed_chapter_with_page(&h.app, &cookie, manga_id, 1).await; + + let resp = h + .app + .clone() + .oneshot(common::post_json_with_cookie( + &format!("/api/v1/mangas/{manga_id}/tags"), + json!({ "name": "Funny" }), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::CREATED); + + assert_eq!(add_tag(&h.app, &cookie, &page_id, "funny").await, StatusCode::CREATED); + + let (tag_count,): (i64,) = + sqlx::query_as("SELECT count(*) FROM tags WHERE lower(name) = 'funny'") + .fetch_one(&db) + .await + .unwrap(); + assert_eq!(tag_count, 1, "page tag and manga tag must share one tags row"); + + let (shared,): (bool,) = sqlx::query_as( + "SELECT (SELECT tag_id FROM page_tags LIMIT 1) \ + = (SELECT tag_id FROM manga_tags LIMIT 1)", + ) + .fetch_one(&db) + .await + .unwrap(); + assert!( + shared, + "page_tags.tag_id must reference the same row as manga_tags.tag_id" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn adding_a_page_tag_creates_a_shared_tags_row(pool: PgPool) { + // Even with no manga tag in play, adding a page tag must populate + // the shared `tags` lookup table (one row per distinct name). + let db = pool.clone(); + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + let manga_id = common::seed_manga_via_api(&h.app, &cookie, "M").await; + let (_, page_id) = seed_chapter_with_page(&h.app, &cookie, manga_id, 1).await; + + assert_eq!(add_tag(&h.app, &cookie, &page_id, "funny").await, StatusCode::CREATED); + + let (name,): (String,) = + sqlx::query_as("SELECT name FROM tags WHERE lower(name) = 'funny'") + .fetch_one(&db) + .await + .unwrap(); + assert_eq!(name, "funny"); +} + #[sqlx::test(migrations = "./migrations")] async fn tags_require_authentication(pool: PgPool) { let h = common::harness(pool); diff --git a/frontend/package.json b/frontend/package.json index cfe5ac8..d64fc9d 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.63.0", + "version": "0.64.0", "private": true, "type": "module", "scripts": {