diff --git a/backend/src/api/page_tags.rs b/backend/src/api/page_tags.rs index 8beabe6..b27e2a0 100644 --- a/backend/src/api/page_tags.rs +++ b/backend/src/api/page_tags.rs @@ -392,10 +392,9 @@ pub struct AggregateParams { pub limit: i64, #[serde(default)] pub offset: i64, - /// Reserved for the planned OCR text-search input. Accepted on - /// the wire so adding OCR later won't break the API shape, but - /// rejected with 501 `text_search_not_yet_supported` if non-empty - /// until the backend supports it. + /// OCR text filter. When non-empty, only pages whose analysis + /// `search_doc` matches the query (`plainto_tsquery`) are aggregated. + /// Blank/absent ⇒ tag-only aggregation. #[serde(default)] pub text: Option, } @@ -411,32 +410,17 @@ fn parse_order(raw: Option<&str>) -> AppResult { } } -fn ensure_text_unsupported(text: Option<&str>) -> AppResult<()> { - // Future OCR search will plug in here. Until then, return a - // distinct code (`text_search_not_yet_supported`) so clients can - // detect "feature pending" vs. a generic 4xx — the code is the - // wire contract, not the message. - if text.is_some_and(|s| !s.trim().is_empty()) { - return Err(AppError::NotImplemented { - code: "text_search_not_yet_supported", - message: "text search is reserved for the planned OCR input but not yet supported", - }); - } - Ok(()) -} - async fn list_chapters_for_tag( State(state): State, CurrentUser(user): CurrentUser, Query(params): Query, ) -> AppResult>> { - ensure_text_unsupported(params.text.as_deref())?; let tag = normalize_tag(¶ms.tag)?; let order = parse_order(params.order.as_deref())?; let limit = params.limit.clamp(1, 200); let offset = params.offset.max(0); let (items, total) = repo::page_tag::aggregate_chapters_for_tag( - &state.db, user.id, &tag, order, limit, offset, + &state.db, user.id, &tag, order, limit, offset, params.text.as_deref(), ) .await?; Ok(Json(PagedResponse::with_total(items, limit, offset, total))) @@ -447,13 +431,12 @@ async fn list_mangas_for_tag( CurrentUser(user): CurrentUser, Query(params): Query, ) -> AppResult>> { - ensure_text_unsupported(params.text.as_deref())?; let tag = normalize_tag(¶ms.tag)?; let order = parse_order(params.order.as_deref())?; let limit = params.limit.clamp(1, 200); let offset = params.offset.max(0); let (items, total) = repo::page_tag::aggregate_mangas_for_tag( - &state.db, user.id, &tag, order, limit, offset, + &state.db, user.id, &tag, order, limit, offset, params.text.as_deref(), ) .await?; Ok(Json(PagedResponse::with_total(items, limit, offset, total))) diff --git a/backend/src/error.rs b/backend/src/error.rs index 630a8cf..6253893 100644 --- a/backend/src/error.rs +++ b/backend/src/error.rs @@ -39,10 +39,10 @@ pub enum AppError { details: serde_json::Value, }, /// 501 — the wire shape is accepted but the feature isn't built yet. - /// Carries a `&'static str` snake_case code so clients can detect - /// the specific pending feature (`text_search_not_yet_supported`, - /// etc.) without parsing the message. Used today by the `?text=` - /// reservation on the page-tag aggregation endpoints. + /// Carries a `&'static str` snake_case code so clients can detect the + /// specific pending feature without parsing the message. Generic; kept + /// for future reservations (the `?text=` page-tag reservation that first + /// used it now performs real OCR search). #[error("not implemented: {code}")] NotImplemented { code: &'static str, diff --git a/backend/src/repo/page_tag.rs b/backend/src/repo/page_tag.rs index 755fd47..0981712 100644 --- a/backend/src/repo/page_tag.rs +++ b/backend/src/repo/page_tag.rs @@ -247,6 +247,11 @@ pub async fn distinct_tags_for_user( /// storage keys (page-number ascending) so the row can render a /// thumbnail strip without a follow-up fetch. /// +/// When `text` is non-blank, results are further restricted to pages whose +/// analysis `search_doc` matches the query (OCR text search), and both +/// `match_count` and the sample thumbnails reflect that filtered set — +/// i.e. `match_count` counts tagged pages whose OCR also matches `text`. +/// /// `order` is inlined via `format!()` — the enum value space is /// closed (`ASC` / `DESC`) so this is not a SQL-injection vector. pub async fn aggregate_chapters_for_tag( @@ -256,7 +261,30 @@ pub async fn aggregate_chapters_for_tag( order: Order, limit: i64, offset: i64, + text: Option<&str>, ) -> AppResult<(Vec, i64)> { + // OCR text filter: when present, additionally require the page's analysis + // `search_doc` to match the query. Reuses the precomputed tsvector exactly + // like `repo::page_analysis::page_search`. `None`/blank ⇒ tag-only. + let text = text.map(str::trim).filter(|s| !s.is_empty()); + let (text_join, text_where) = match text { + // `$5` in the main query, `$3` in the count query (see binds below). + Some(_) => ( + "JOIN page_analysis pa ON pa.page_id = p.id", + "AND pa.search_doc @@ plainto_tsquery('simple', {n})", + ), + None => ("", ""), + }; + // Same filter inside the correlated sample-thumbnail subquery (its page is + // aliased `p`), so the thumbnails match what the text search matched. The + // subquery lives in the main query, so it reuses the `$5` text bind. + let (sample_join, sample_where) = match text { + Some(_) => ( + "JOIN page_analysis pa2 ON pa2.page_id = p.id", + "AND pa2.search_doc @@ plainto_tsquery('simple', $5)", + ), + None => ("", ""), + }; let sql = format!( r#" SELECT @@ -274,9 +302,11 @@ pub async fn aggregate_chapters_for_tag( FROM pages p JOIN page_tags pt2 ON pt2.page_id = p.id JOIN tags t2 ON t2.id = pt2.tag_id + {sample_join} WHERE p.chapter_id = ch.id AND pt2.user_id = $1 AND lower(t2.name) = $2 + {sample_where} ORDER BY p.page_number ASC LIMIT 3 ) p2 @@ -288,43 +318,60 @@ pub async fn aggregate_chapters_for_tag( 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 + {text_join} WHERE pt.user_id = $1 AND lower(t.name) = $2 + {text_where} GROUP BY ch.id, ch.manga_id, m.title, ch.number, ch.title ORDER BY match_count {dir}, ch.id LIMIT $3 OFFSET $4 "#, dir = order.as_sql(), + text_join = text_join, + text_where = text_where.replace("{n}", "$5"), + sample_join = sample_join, + sample_where = sample_where, ); - let rows = sqlx::query_as::<_, TaggedChapterAggregate>(&sql) + let mut q = sqlx::query_as::<_, TaggedChapterAggregate>(&sql) .bind(user_id) .bind(tag) .bind(limit) - .bind(offset) - .fetch_all(pool) - .await?; + .bind(offset); + if let Some(text) = text { + q = q.bind(text); + } + let rows = q.fetch_all(pool).await?; - let (total,): (i64,) = sqlx::query_as( + let count_sql = format!( r#" 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 + {text_join} WHERE pt.user_id = $1 AND lower(t.name) = $2 + {text_where} GROUP BY p.chapter_id ) c "#, - ) - .bind(user_id) - .bind(tag) - .fetch_one(pool) - .await?; + text_join = text_join, + text_where = text_where.replace("{n}", "$3"), + ); + let mut cq = sqlx::query_as::<_, (i64,)>(&count_sql).bind(user_id).bind(tag); + if let Some(text) = text { + cq = cq.bind(text); + } + let (total,) = cq.fetch_one(pool).await?; Ok((rows, total)) } /// Paged list of mangas containing pages tagged `tag` for `user_id`, /// ranked by `match_count` summed across all their chapters. +/// +/// `text` behaves as in [`aggregate_chapters_for_tag`]: non-blank restricts to +/// pages whose OCR `search_doc` matches, and both `match_count` and the sample +/// thumbnails reflect that filtered set. pub async fn aggregate_mangas_for_tag( pool: &PgPool, user_id: Uuid, @@ -332,7 +379,26 @@ pub async fn aggregate_mangas_for_tag( order: Order, limit: i64, offset: i64, + text: Option<&str>, ) -> AppResult<(Vec, i64)> { + // OCR text filter — see `aggregate_chapters_for_tag` for the rationale. + let text = text.map(str::trim).filter(|s| !s.is_empty()); + let (text_join, text_where) = match text { + Some(_) => ( + "JOIN page_analysis pa ON pa.page_id = p.id", + "AND pa.search_doc @@ plainto_tsquery('simple', {n})", + ), + None => ("", ""), + }; + // Same filter inside the sample-thumbnail subquery (page aliased `p`), + // reusing the main query's `$5` text bind. + let (sample_join, sample_where) = match text { + Some(_) => ( + "JOIN page_analysis pa2 ON pa2.page_id = p.id", + "AND pa2.search_doc @@ plainto_tsquery('simple', $5)", + ), + None => ("", ""), + }; let sql = format!( r#" SELECT @@ -349,9 +415,11 @@ pub async fn aggregate_mangas_for_tag( 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 + {sample_join} WHERE ch2.manga_id = m.id AND pt2.user_id = $1 AND lower(t2.name) = $2 + {sample_where} ORDER BY p.page_number ASC LIMIT 3 ) p2 @@ -363,23 +431,31 @@ pub async fn aggregate_mangas_for_tag( 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 + {text_join} WHERE pt.user_id = $1 AND lower(t.name) = $2 + {text_where} GROUP BY m.id, m.title, m.cover_image_path ORDER BY match_count {dir}, m.id LIMIT $3 OFFSET $4 "#, dir = order.as_sql(), + text_join = text_join, + text_where = text_where.replace("{n}", "$5"), + sample_join = sample_join, + sample_where = sample_where, ); - let rows = sqlx::query_as::<_, TaggedMangaAggregate>(&sql) + let mut q = sqlx::query_as::<_, TaggedMangaAggregate>(&sql) .bind(user_id) .bind(tag) .bind(limit) - .bind(offset) - .fetch_all(pool) - .await?; + .bind(offset); + if let Some(text) = text { + q = q.bind(text); + } + let rows = q.fetch_all(pool).await?; - let (total,): (i64,) = sqlx::query_as( + let count_sql = format!( r#" SELECT count(*) FROM ( SELECT 1 @@ -387,15 +463,20 @@ pub async fn aggregate_mangas_for_tag( 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 + {text_join} WHERE pt.user_id = $1 AND lower(t.name) = $2 + {text_where} GROUP BY ch.manga_id ) m "#, - ) - .bind(user_id) - .bind(tag) - .fetch_one(pool) - .await?; + text_join = text_join, + text_where = text_where.replace("{n}", "$3"), + ); + let mut cq = sqlx::query_as::<_, (i64,)>(&count_sql).bind(user_id).bind(tag); + if let Some(text) = text { + cq = cq.bind(text); + } + let (total,) = cq.fetch_one(pool).await?; Ok((rows, total)) } diff --git a/backend/tests/api_page_tags.rs b/backend/tests/api_page_tags.rs index c855254..6bb19a2 100644 --- a/backend/tests/api_page_tags.rs +++ b/backend/tests/api_page_tags.rs @@ -680,24 +680,123 @@ async fn aggregate_rejects_invalid_order(pool: PgPool) { } #[sqlx::test(migrations = "./migrations")] -async fn aggregate_with_text_param_is_501_with_stable_code(pool: PgPool) { - // OCR text search isn't built yet; the param is accepted so adding - // OCR won't break the wire shape, but rejected with a distinct - // status + code. The code is the wire contract — clients pin on - // `text_search_not_yet_supported`, not the message. - let h = common::harness(pool); +async fn aggregate_with_text_param_filters_by_ocr(pool: PgPool) { + // OCR text search: a page tagged `funny` whose OCR contains "guts" is + // returned for `&text=guts` on both aggregation endpoints, and excluded + // for a query its OCR doesn't contain. The filter runs against the same + // precomputed `search_doc` the OCR worker writes via `persist_analysis`. + let h = common::harness(pool.clone()); 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); + + // Persist OCR text exactly as the ocrs backend would (via the same mapper). + let page_uuid = Uuid::parse_str(&page_id).unwrap(); + let analysis = + mangalord::analysis::ocr::lines_to_analysis(vec!["spilling the guts here".to_string()]); + mangalord::repo::page_analysis::persist_analysis(&pool, page_uuid, &analysis, "ocrs") + .await + .unwrap(); + + for endpoint in ["chapters", "mangas"] { + // Matching text → the tagged+OCR'd row is returned. + let resp = h + .app + .clone() + .oneshot(common::get_with_cookie( + &format!("/api/v1/me/page-tags/{endpoint}?tag=funny&text=guts"), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK, "{endpoint} matching"); + let body = common::body_json(resp).await; + assert_eq!(body["items"].as_array().unwrap().len(), 1, "{endpoint} matching items"); + assert_eq!(body["page"]["total"], 1, "{endpoint} matching total"); + + // Non-matching text → excluded. + let resp = h + .app + .clone() + .oneshot(common::get_with_cookie( + &format!("/api/v1/me/page-tags/{endpoint}?tag=funny&text=zzzznomatch"), + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK, "{endpoint} non-matching"); + let body = common::body_json(resp).await; + assert_eq!(body["items"].as_array().unwrap().len(), 0, "{endpoint} non-matching items"); + assert_eq!(body["page"]["total"], 0, "{endpoint} non-matching total"); + } +} + +#[sqlx::test(migrations = "./migrations")] +async fn aggregate_text_param_counts_only_matching_pages(pool: PgPool) { + // A chapter with TWO tagged pages where only ONE page's OCR matches the + // text. `match_count` / `total` must reflect the filtered count (1), not + // the tag-only count (2), and the sample thumbnails must contain only the + // matching page. This is what a single-page test can't distinguish — it + // pins the JOIN/placeholder wiring and the sample-subquery filter. + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + let manga_id = common::seed_manga_via_api(&h.app, &cookie, "M").await; + let (_, page_ids) = seed_chapter_with_n_pages(&h.app, &cookie, manga_id, 1, 2).await; + for pid in &page_ids { + assert_eq!(add_tag(&h.app, &cookie, pid, "funny").await, StatusCode::CREATED); + } + + // Page 0 OCR contains "guts"; page 1 OCR contains only "filler". + let p0 = Uuid::parse_str(&page_ids[0]).unwrap(); + let p1 = Uuid::parse_str(&page_ids[1]).unwrap(); + let a0 = mangalord::analysis::ocr::lines_to_analysis(vec!["the guts spill out".to_string()]); + let a1 = mangalord::analysis::ocr::lines_to_analysis(vec!["just filler text".to_string()]); + mangalord::repo::page_analysis::persist_analysis(&pool, p0, &a0, "ocrs").await.unwrap(); + mangalord::repo::page_analysis::persist_analysis(&pool, p1, &a1, "ocrs").await.unwrap(); + let resp = h .app + .clone() .oneshot(common::get_with_cookie( "/api/v1/me/page-tags/chapters?tag=funny&text=guts", &cookie, )) .await .unwrap(); - assert_eq!(resp.status(), StatusCode::NOT_IMPLEMENTED); + assert_eq!(resp.status(), StatusCode::OK); let body = common::body_json(resp).await; - assert_eq!(body["error"]["code"], "text_search_not_yet_supported"); + let items = body["items"].as_array().unwrap(); + assert_eq!(items.len(), 1, "one chapter row"); + assert_eq!(body["page"]["total"], 1); + // Filtered match_count is the matching-page count, not the tag count. + assert_eq!(items[0]["match_count"], 1, "only one page's OCR matches"); + // Sample thumbnails reflect the filter: only the matching page's key. + let samples = items[0]["sample_storage_keys"].as_array().unwrap(); + assert_eq!(samples.len(), 1, "thumbnails restricted to matching pages"); +} + +#[sqlx::test(migrations = "./migrations")] +async fn aggregate_blank_text_param_is_tag_only(pool: PgPool) { + // A blank `text=` must not filter — it falls back to tag-only aggregation + // (the page has a tag but no analysis row at all). + 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 resp = h + .app + .oneshot(common::get_with_cookie( + "/api/v1/me/page-tags/chapters?tag=funny&text=", + &cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = common::body_json(resp).await; + assert_eq!(body["items"].as_array().unwrap().len(), 1); } #[sqlx::test(migrations = "./migrations")]