feat: route reader by chapter id, allow duplicate-numbered chapters (0.24.0)
Real-world sources publish multiple chapters at the same number:
different scanlators ("Ch.52 from bloomingdale" + "Ch.52 from mina"),
translator notices and farewells, alt-translations. The (manga_id,
number) UNIQUE constraint from 0001 silently collapsed all of those
into a single row via the upsert path in repo::crawler. Migration 0013
drops the constraint; sync_manga_chapters now plain-INSERTs each
SourceChapterRef so every parsed chapter survives as its own row.
Identity moves from the (manga_id, number) tuple to the chapter UUID:
- `GET /api/v1/mangas/:manga_id/chapters/:chapter_id` (replaces :number)
- `GET /api/v1/mangas/:manga_id/chapters/:chapter_id/pages`
- `repo::chapter::find_by_id_in_manga` (replaces find_by_manga_and_number)
- Frontend reader route renamed to `/manga/[id]/chapter/[chapter_id]`
- Chapter links throughout (manga page list, continue-reading CTA,
reader prev/next, history rows, bookmark cards) use chapter.id
- API clients getChapter/getChapterPages take a chapter id string
read_progress + bookmarks already FK chapter_id; they only enrich with
chapter_number for display, which is preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1415,7 +1415,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.23.1"
|
||||
version = "0.24.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.23.1"
|
||||
version = "0.24.0"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
18
backend/migrations/0013_drop_chapters_unique_number.sql
Normal file
18
backend/migrations/0013_drop_chapters_unique_number.sql
Normal file
@@ -0,0 +1,18 @@
|
||||
-- Real-world sources publish multiple chapters at the same number:
|
||||
-- different uploaders, translator notices/farewells, paid-vs-free
|
||||
-- re-uploads, and our own users can legitimately have two versions of
|
||||
-- "Ch.52" with different scanlations. The (manga_id, number) UNIQUE
|
||||
-- from 0001_init silently collapses all of those into a single row via
|
||||
-- ON CONFLICT, dropping data. Drop the constraint and lean on the
|
||||
-- chapter id (UUID) as the only chapter identity going forward.
|
||||
|
||||
ALTER TABLE chapters DROP CONSTRAINT chapters_manga_id_number_key;
|
||||
|
||||
-- The UNIQUE was also our only index on (manga_id, number) since
|
||||
-- 0007 dropped the redundant explicit one. Chapter list pages
|
||||
-- ORDER BY number ASC and the manga page is a hot read path, so put
|
||||
-- the index back without the uniqueness. Secondary sort by created_at
|
||||
-- so duplicate-numbered chapters have a stable order in lists and
|
||||
-- prev/next navigation.
|
||||
CREATE INDEX chapters_manga_id_number_idx
|
||||
ON chapters (manga_id, number, created_at);
|
||||
@@ -26,9 +26,9 @@ use crate::upload::{parse_image, UploadedImage};
|
||||
pub fn routes() -> Router<AppState> {
|
||||
Router::new()
|
||||
.route("/mangas/:manga_id/chapters", get(list).post(create))
|
||||
.route("/mangas/:manga_id/chapters/:number", get(get_one))
|
||||
.route("/mangas/:manga_id/chapters/:chapter_id", get(get_one))
|
||||
.route(
|
||||
"/mangas/:manga_id/chapters/:number/pages",
|
||||
"/mangas/:manga_id/chapters/:chapter_id/pages",
|
||||
get(list_pages),
|
||||
)
|
||||
}
|
||||
@@ -60,10 +60,10 @@ async fn list(
|
||||
|
||||
async fn get_one(
|
||||
State(state): State<AppState>,
|
||||
Path((manga_id, number)): Path<(Uuid, i32)>,
|
||||
Path((manga_id, chapter_id)): Path<(Uuid, Uuid)>,
|
||||
) -> AppResult<Json<Chapter>> {
|
||||
repo::manga::get(&state.db, manga_id).await?;
|
||||
let chapter = repo::chapter::find_by_manga_and_number(&state.db, manga_id, number)
|
||||
let chapter = repo::chapter::find_by_id_in_manga(&state.db, manga_id, chapter_id)
|
||||
.await?
|
||||
.ok_or(AppError::NotFound)?;
|
||||
Ok(Json(chapter))
|
||||
@@ -164,10 +164,10 @@ struct PagesResponse {
|
||||
|
||||
async fn list_pages(
|
||||
State(state): State<AppState>,
|
||||
Path((manga_id, number)): Path<(Uuid, i32)>,
|
||||
Path((manga_id, chapter_id)): Path<(Uuid, Uuid)>,
|
||||
) -> AppResult<Json<PagesResponse>> {
|
||||
repo::manga::get(&state.db, manga_id).await?;
|
||||
let chapter = repo::chapter::find_by_manga_and_number(&state.db, manga_id, number)
|
||||
let chapter = repo::chapter::find_by_id_in_manga(&state.db, manga_id, chapter_id)
|
||||
.await?
|
||||
.ok_or(AppError::NotFound)?;
|
||||
let pages = repo::page::list_for_chapter(&state.db, chapter.id).await?;
|
||||
|
||||
@@ -12,12 +12,15 @@ pub async fn list_for_manga(
|
||||
limit: i64,
|
||||
offset: i64,
|
||||
) -> AppResult<Vec<Chapter>> {
|
||||
// Secondary sort by created_at gives duplicate-numbered chapters
|
||||
// (multiple uploaders/translations of the same number) a stable
|
||||
// order in lists and prev/next reader navigation.
|
||||
let rows = sqlx::query_as::<_, Chapter>(
|
||||
r#"
|
||||
SELECT id, manga_id, number, title, page_count, created_at
|
||||
FROM chapters
|
||||
WHERE manga_id = $1
|
||||
ORDER BY number ASC
|
||||
ORDER BY number ASC, created_at ASC
|
||||
LIMIT $2 OFFSET $3
|
||||
"#,
|
||||
)
|
||||
@@ -29,33 +32,40 @@ pub async fn list_for_manga(
|
||||
Ok(rows)
|
||||
}
|
||||
|
||||
pub async fn find_by_manga_and_number(
|
||||
/// Look up a chapter by its UUID, scoped to its manga so a UUID guessed
|
||||
/// from a different manga's URL doesn't accidentally resolve.
|
||||
pub async fn find_by_id_in_manga(
|
||||
pool: &PgPool,
|
||||
manga_id: Uuid,
|
||||
number: i32,
|
||||
chapter_id: Uuid,
|
||||
) -> AppResult<Option<Chapter>> {
|
||||
let row = sqlx::query_as::<_, Chapter>(
|
||||
r#"
|
||||
SELECT id, manga_id, number, title, page_count, created_at
|
||||
FROM chapters
|
||||
WHERE manga_id = $1 AND number = $2
|
||||
WHERE manga_id = $1 AND id = $2
|
||||
"#,
|
||||
)
|
||||
.bind(manga_id)
|
||||
.bind(number)
|
||||
.bind(chapter_id)
|
||||
.fetch_optional(pool)
|
||||
.await?;
|
||||
Ok(row)
|
||||
}
|
||||
|
||||
/// Accepts any `PgExecutor` so the upload handler can run this inside a
|
||||
/// transaction with the per-page inserts. Returns `AppError::Conflict`
|
||||
/// on the (manga_id, number) unique violation so handlers can surface a
|
||||
/// clean 409.
|
||||
/// transaction with the per-page inserts.
|
||||
///
|
||||
/// `uploaded_by` records who uploaded the chapter and feeds the
|
||||
/// per-user upload history. `None` means "historical / API token with
|
||||
/// no associated user" — kept nullable to support that case.
|
||||
///
|
||||
/// Chapter identity is the row UUID; the same (manga_id, number)
|
||||
/// combination can repeat (multiple translations, re-uploads). The
|
||||
/// `is_unique_violation` branch below is a defensive holdover from
|
||||
/// 0001's (manga_id, number) UNIQUE — it can no longer fire under
|
||||
/// normal operation, but we surface a clean 409 if a future migration
|
||||
/// re-adds any chapter uniqueness.
|
||||
pub async fn create<'e, E: PgExecutor<'e>>(
|
||||
executor: E,
|
||||
manga_id: Uuid,
|
||||
@@ -80,7 +90,7 @@ pub async fn create<'e, E: PgExecutor<'e>>(
|
||||
match result {
|
||||
Ok(c) => Ok(c),
|
||||
Err(e) if is_unique_violation(&e) => Err(AppError::Conflict(format!(
|
||||
"chapter {number} already exists for this manga"
|
||||
"chapter {number} conflicts with an existing chapter for this manga"
|
||||
))),
|
||||
Err(e) => Err(AppError::Database(e)),
|
||||
}
|
||||
|
||||
@@ -332,15 +332,15 @@ pub async fn sync_manga_chapters(
|
||||
|
||||
match existing {
|
||||
None => {
|
||||
// New chapter row. The (manga_id, number) unique
|
||||
// constraint protects against re-inserts if the same
|
||||
// number arrives via a different source_chapter_key.
|
||||
// New chapter row. As of 0013 there's no (manga_id,
|
||||
// number) UNIQUE, so duplicate-numbered chapters from
|
||||
// the source (different uploaders, notices, alt
|
||||
// translations) each get their own row — chapter
|
||||
// identity is the UUID, not the number.
|
||||
let (chapter_id,): (Uuid,) = sqlx::query_as(
|
||||
r#"
|
||||
INSERT INTO chapters (manga_id, number, title, page_count)
|
||||
VALUES ($1, $2, $3, 0)
|
||||
ON CONFLICT (manga_id, number) DO UPDATE
|
||||
SET title = EXCLUDED.title
|
||||
RETURNING id
|
||||
"#,
|
||||
)
|
||||
|
||||
@@ -12,12 +12,18 @@ async fn seed_manga(h: &common::Harness, cookie: &str, title: &str) -> Uuid {
|
||||
common::seed_manga_via_api(&h.app, cookie, title).await
|
||||
}
|
||||
|
||||
async fn seed_chapter(pool: &PgPool, manga_id: Uuid, number: i32, title: Option<&str>) {
|
||||
async fn seed_chapter(
|
||||
pool: &PgPool,
|
||||
manga_id: Uuid,
|
||||
number: i32,
|
||||
title: Option<&str>,
|
||||
) -> Uuid {
|
||||
// Historical seed — uploaded_by remains NULL, mirroring the
|
||||
// pre-Phase-5 rows in the production DB.
|
||||
mangalord::repo::chapter::create(pool, manga_id, number, title, None)
|
||||
.await
|
||||
.unwrap();
|
||||
.unwrap()
|
||||
.id
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
@@ -81,16 +87,16 @@ async fn list_chapters_returns_404_for_unknown_manga(pool: PgPool) {
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn get_chapter_by_number(pool: PgPool) {
|
||||
async fn get_chapter_by_id(pool: PgPool) {
|
||||
let h = common::harness(pool.clone());
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = seed_manga(&h, &cookie, "Berserk").await;
|
||||
seed_chapter(&pool, manga_id, 1, Some("The Brand")).await;
|
||||
let chapter_id = seed_chapter(&pool, manga_id, 1, Some("The Brand")).await;
|
||||
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_id}/chapters/1"
|
||||
"/api/v1/mangas/{manga_id}/chapters/{chapter_id}"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -99,18 +105,20 @@ async fn get_chapter_by_number(pool: PgPool) {
|
||||
assert_eq!(body["number"], 1);
|
||||
assert_eq!(body["title"], "The Brand");
|
||||
assert_eq!(body["page_count"], 0);
|
||||
assert_eq!(body["id"], chapter_id.to_string());
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn get_chapter_unknown_number_is_404(pool: PgPool) {
|
||||
async fn get_chapter_unknown_id_is_404(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = seed_manga(&h, &cookie, "Berserk").await;
|
||||
let unknown_chapter = Uuid::new_v4();
|
||||
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_id}/chapters/99"
|
||||
"/api/v1/mangas/{manga_id}/chapters/{unknown_chapter}"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -122,10 +130,34 @@ async fn get_chapter_unknown_number_is_404(pool: PgPool) {
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn get_chapter_unknown_manga_is_404(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let unknown = Uuid::nil();
|
||||
let unknown_manga = Uuid::nil();
|
||||
let unknown_chapter = Uuid::new_v4();
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!("/api/v1/mangas/{unknown}/chapters/1")))
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{unknown_manga}/chapters/{unknown_chapter}"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
|
||||
}
|
||||
|
||||
/// Cross-manga isolation: a chapter id belonging to manga A must not
|
||||
/// resolve when accessed via manga B's URL. The (manga_id, id) scoping
|
||||
/// in `find_by_id_in_manga` enforces this.
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn get_chapter_from_wrong_manga_is_404(pool: PgPool) {
|
||||
let h = common::harness(pool.clone());
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_a = seed_manga(&h, &cookie, "Berserk").await;
|
||||
let manga_b = seed_manga(&h, &cookie, "Vagabond").await;
|
||||
let chapter_id = seed_chapter(&pool, manga_a, 1, Some("Episode 1")).await;
|
||||
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_b}/chapters/{chapter_id}"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
|
||||
@@ -136,12 +168,12 @@ async fn list_pages_empty_for_chapter_without_upload(pool: PgPool) {
|
||||
let h = common::harness(pool.clone());
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = seed_manga(&h, &cookie, "Berserk").await;
|
||||
seed_chapter(&pool, manga_id, 1, None).await;
|
||||
let chapter_id = seed_chapter(&pool, manga_id, 1, None).await;
|
||||
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_id}/chapters/1/pages"
|
||||
"/api/v1/mangas/{manga_id}/chapters/{chapter_id}/pages"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -155,11 +187,12 @@ async fn list_pages_returns_404_for_unknown_chapter(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = seed_manga(&h, &cookie, "Berserk").await;
|
||||
let unknown_chapter = Uuid::new_v4();
|
||||
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_id}/chapters/99/pages"
|
||||
"/api/v1/mangas/{manga_id}/chapters/{unknown_chapter}/pages"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -139,13 +139,17 @@ async fn files_endpoint_streams_in_multiple_frames(pool: PgPool) {
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::CREATED);
|
||||
let chapter_id = common::body_json(resp).await["id"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.to_string();
|
||||
|
||||
// Fetch the page back via the streaming files endpoint.
|
||||
let pages = h
|
||||
.app
|
||||
.clone()
|
||||
.oneshot(common::get(&format!(
|
||||
"/api/v1/mangas/{manga_id}/chapters/1/pages"
|
||||
"/api/v1/mangas/{manga_id}/chapters/{chapter_id}/pages"
|
||||
)))
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -317,8 +321,12 @@ async fn create_chapter_rejects_renamed_non_image_page(pool: PgPool) {
|
||||
assert_eq!(body["error"]["code"], "unsupported_media_type");
|
||||
}
|
||||
|
||||
/// Multiple chapters can share the same number — different
|
||||
/// scanlations, re-uploads, translator notes. As of migration 0013,
|
||||
/// (manga_id, number) is not unique and each upload gets its own
|
||||
/// chapter id.
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn create_chapter_returns_409_on_duplicate_number(pool: PgPool) {
|
||||
async fn create_chapter_allows_duplicate_numbers_as_separate_chapters(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await;
|
||||
@@ -334,10 +342,27 @@ async fn create_chapter_returns_409_on_duplicate_number(pool: PgPool) {
|
||||
};
|
||||
let first = h.app.clone().oneshot(make()).await.unwrap();
|
||||
assert_eq!(first.status(), StatusCode::CREATED);
|
||||
let second = h.app.oneshot(make()).await.unwrap();
|
||||
assert_eq!(second.status(), StatusCode::CONFLICT);
|
||||
let body = common::body_json(second).await;
|
||||
assert_eq!(body["error"]["code"], "conflict");
|
||||
let first_id = common::body_json(first).await["id"].as_str().unwrap().to_string();
|
||||
|
||||
let second = h.app.clone().oneshot(make()).await.unwrap();
|
||||
assert_eq!(second.status(), StatusCode::CREATED);
|
||||
let second_id = common::body_json(second).await["id"].as_str().unwrap().to_string();
|
||||
|
||||
assert_ne!(first_id, second_id, "each upload gets a distinct chapter id");
|
||||
|
||||
// List endpoint surfaces both rows.
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::get(&format!("/api/v1/mangas/{manga_id}/chapters")))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
let body = common::body_json(resp).await;
|
||||
let items = body["items"].as_array().unwrap();
|
||||
assert_eq!(items.len(), 2, "both Ch.1 uploads listed separately");
|
||||
for item in items {
|
||||
assert_eq!(item["number"], 1);
|
||||
}
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
|
||||
@@ -232,6 +232,82 @@ async fn sync_chapters_adds_new_refreshes_existing_and_drops_vanished(pool: PgPo
|
||||
assert!(dropped.0.is_some(), "ch2 should be soft-dropped");
|
||||
}
|
||||
|
||||
/// Real-world sources publish multiple chapters at the same number
|
||||
/// (different uploaders, translator notes, re-releases). After the
|
||||
/// (manga_id, number) UNIQUE drop in 0013, each `SourceChapterRef`
|
||||
/// becomes its own `chapters` row even when the parsed number matches
|
||||
/// — chapter identity is now the chapter id, not the number.
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn sync_chapters_keeps_duplicate_numbered_chapters_as_separate_rows(pool: PgPool) {
|
||||
crawler::ensure_source(&pool, "target", "T", "https://x.example")
|
||||
.await
|
||||
.unwrap();
|
||||
let m = sample_manga("foo", "Foo Manga", "hash-1");
|
||||
let up = crawler::upsert_manga_from_source(&pool, "target", "https://x.example/foo", &m)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Two distinct uploads of Ch.52 (different uploaders → different
|
||||
// URLs/keys, same parsed number) plus a notice/hiatus row that
|
||||
// parses to number=0 alongside a real chapter at number 1.
|
||||
let chapters = vec![
|
||||
SourceChapterRef {
|
||||
source_chapter_key: "br_chapter-A".into(),
|
||||
number: 52,
|
||||
title: Some("Ch.52 : Official".into()),
|
||||
url: "https://x.example/foo/A/pg-1/".into(),
|
||||
},
|
||||
SourceChapterRef {
|
||||
source_chapter_key: "br_chapter-B".into(),
|
||||
number: 52,
|
||||
title: Some("Ch.52 : Official (alt)".into()),
|
||||
url: "https://x.example/foo/B/pg-1/".into(),
|
||||
},
|
||||
SourceChapterRef {
|
||||
source_chapter_key: "br_chapter-NOTICE".into(),
|
||||
number: 0,
|
||||
title: Some("hitaus.".into()),
|
||||
url: "https://x.example/foo/notice/pg-1/".into(),
|
||||
},
|
||||
SourceChapterRef {
|
||||
source_chapter_key: "br_chapter-1".into(),
|
||||
number: 1,
|
||||
title: Some("Ch.1 : Official".into()),
|
||||
url: "https://x.example/foo/1/pg-1/".into(),
|
||||
},
|
||||
];
|
||||
|
||||
let diff = crawler::sync_manga_chapters(&pool, "target", up.manga_id, &chapters)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
diff,
|
||||
ChapterDiff {
|
||||
new: 4,
|
||||
refreshed: 0,
|
||||
dropped: 0
|
||||
},
|
||||
"every source ref yields a new chapter row"
|
||||
);
|
||||
|
||||
let rows: (i64,) =
|
||||
sqlx::query_as("SELECT COUNT(*) FROM chapters WHERE manga_id = $1")
|
||||
.bind(up.manga_id)
|
||||
.fetch_one(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(rows.0, 4, "4 distinct chapter rows even with duplicate numbers");
|
||||
|
||||
let ch52_count: (i64,) = sqlx::query_as(
|
||||
"SELECT COUNT(*) FROM chapters WHERE manga_id = $1 AND number = 52",
|
||||
)
|
||||
.bind(up.manga_id)
|
||||
.fetch_one(&pool)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(ch52_count.0, 2, "both Ch.52 uploads survive as separate rows");
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn mark_dropped_mangas_only_drops_unseen(pool: PgPool) {
|
||||
crawler::ensure_source(&pool, "target", "T", "https://x.example")
|
||||
|
||||
Reference in New Issue
Block a user