From 3564f170b4451b7cff8fe04740617aa551271b1b Mon Sep 17 00:00:00 2001 From: Manielle <75130972+Manielle@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:36:04 +0200 Subject: [PATCH] Open Library card: a named work (openlibraryid lookup) needs only a title; cover falls back to its ISBN, description optional --- feeder-plugin/book-feeder/src/openlibrary.rs | 143 ++++++++++++++++-- .../book-feeder/src/openlibrary_client.rs | 32 +++- 2 files changed, 160 insertions(+), 15 deletions(-) diff --git a/feeder-plugin/book-feeder/src/openlibrary.rs b/feeder-plugin/book-feeder/src/openlibrary.rs index a8486a2..cbfff2a 100644 --- a/feeder-plugin/book-feeder/src/openlibrary.rs +++ b/feeder-plugin/book-feeder/src/openlibrary.rs @@ -169,14 +169,33 @@ fn foreign_identity_filter(q: &GatewayQuery) -> Option<&str> { /// what passes, permanently. A card that lacks another filter's key does not /// match it, which is the truth. pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Option { + to_record_with(doc, Some(description), query, false) +} + +/// [`to_record`] with the quality bar as a choice. +/// +/// `lenient = false` is the bar every search and browse row keeps: a cover **and** +/// a description, or no card. `lenient = true` is for a lookup that names one work +/// by `openlibraryid` (a work page): the work is already known to be wanted, so a +/// title is enough. Its cover may come from the work's ISBNs when the work has none +/// of its own, and a missing description is simply left out. +pub fn to_record_with( + doc: &SearchDoc, + description: Option<&str>, + query: &GatewayQuery, + lenient: bool, +) -> Option { let id = doc.work_id()?; let cid = card_cid(id)?; let title = doc.title.as_deref().map(str::trim).filter(|t| !t.is_empty())?; - let poster_url = doc.cover_url()?; - let description = description.trim(); - if description.is_empty() { - return None; - } + let description = description.map(str::trim).filter(|d| !d.is_empty()); + let poster_url = if lenient { + doc.cover_url().or_else(|| doc.isbn_cover_url()) + } else { + // Strict: both are required. + description?; + Some(doc.cover_url()?) + }; let mut fields: BTreeMap = BTreeMap::new(); @@ -200,7 +219,9 @@ pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Op // `description/{lang3}`, never a flat `description` — the flat key is indexed // but never written (METADATA_KEYS.md §14.6). - fields.insert("description/eng".into(), description.to_string()); + if let Some(d) = description { + fields.insert("description/eng".into(), d.to_string()); + } let mut seen = HashSet::new(); let authors: Vec<&str> = doc @@ -224,7 +245,9 @@ pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Op // ⚠ `poster_url`, NOT a pre-hashed `poster`: the gateway's SEEDABLE_FIELDS // path fetches it, seeds the bytes and writes the content cid back. - fields.insert("poster_url".into(), poster_url); + if let Some(url) = poster_url { + fields.insert("poster_url".into(), url); + } fields.insert( "sourceUrl".into(), @@ -257,13 +280,14 @@ async fn project( query: &GatewayQuery, n: usize, deadline: Duration, + lenient: bool, ) -> Vec { let mut seen = HashSet::new(); let candidates: Vec = docs .into_iter() .filter(|d| { d.work_id().is_some() - && d.cover_url().is_some() + && (lenient || d.cover_url().is_some()) && d.title.as_deref().is_some_and(|t| !t.trim().is_empty()) }) .filter(|d| seen.insert(d.key.clone())) @@ -286,7 +310,7 @@ async fn project( debug!(from = doc.key, to = w.id, "openlibrary: merged work; skipped"); return None; } - to_record(&doc, w.description.as_deref()?, query) + to_record_with(&doc, w.description.as_deref(), query, lenient) } Ok(None) => None, Err(e) => { @@ -437,7 +461,9 @@ impl FeederPlugin for OpenLibraryPlugin { let Some(doc) = client.doc_by_work_id(id, SEARCH_DEADLINE).await? else { return Ok(Vec::new()); }; - return Ok(project(client, vec![doc], query, 1, SEARCH_DEADLINE).await); + // Lenient: an explicit work lookup keeps the card even without a + // description, and finds a cover by ISBN when the work has none. + return Ok(project(client, vec![doc], query, 1, SEARCH_DEADLINE, true).await); } // A curated row. @@ -455,7 +481,7 @@ impl FeederPlugin for OpenLibraryPlugin { .await? } }; - return Ok(project(client, docs, query, n, BROWSE_DEADLINE).await); + return Ok(project(client, docs, query, n, BROWSE_DEADLINE, false).await); } // Free text. @@ -470,7 +496,7 @@ impl FeederPlugin for OpenLibraryPlugin { let docs = client .search(text, (n * SEARCH_OVERFETCH).min(MAX_LIMIT), SEARCH_DEADLINE) .await?; - Ok(project(client, docs, query, n, SEARCH_DEADLINE).await) + Ok(project(client, docs, query, n, SEARCH_DEADLINE, false).await) } /// A card has no bytes: the record IS the payload. @@ -489,7 +515,8 @@ impl FeederPlugin for OpenLibraryPlugin { // different work (a merge) is not this cid's record. let record = match self.client.as_ref() { Some(c) => match c.doc_by_work_id(id, SEARCH_DEADLINE).await { - Ok(Some(doc)) => project(c, vec![doc], &GatewayQuery::from_free_text(""), 1, SEARCH_DEADLINE) + // Lenient, like the `openlibraryid` lookup that minted this card. + Ok(Some(doc)) => project(c, vec![doc], &GatewayQuery::from_free_text(""), 1, SEARCH_DEADLINE, true) .await .into_iter() .find(|r| r.record_id == id), @@ -545,6 +572,7 @@ mod tests { cover_i: Some(14348537), first_publish_year: Some(1813), id_project_gutenberg: vec![], + isbn: vec![], } } @@ -651,6 +679,95 @@ mod tests { assert!(to_record(&edition, "A novel.", &q("", &[])).is_none()); } + #[test] + fn a_lenient_card_needs_only_a_title_and_finds_a_cover_by_isbn() { + let mut d = pride(); + d.cover_i = None; + d.isbn = vec!["1955252092".into(), "9781955252096".into()]; + // Strict still refuses it, whatever the description. + assert!(to_record_with(&d, Some("A novel."), &q("", &[]), false).is_none()); + // Lenient mints it with the ISBN-13 cover and no description at all. + let r = to_record_with(&d, None, &q("", &[]), true).unwrap(); + assert_eq!( + r.fields["poster_url"], + "https://covers.openlibrary.org/b/isbn/9781955252096-L.jpg?default=false" + ); + assert!(!r.fields.keys().any(|k| k.starts_with("description"))); + assert_eq!(r.fields["openlibraryid"], "OL66554W"); + assert_eq!(r.fields["fileType"], "card"); + // Same address as the strict card for the same work. + let strict = to_record(&pride(), "A novel.", &q("", &[])).unwrap(); + let key = |r: &DiscoveryRecord| r.fields.keys().find(|k| k.starts_with("cids/")).cloned(); + assert_eq!(key(&r), key(&strict)); + } + + #[test] + fn a_lenient_card_with_no_cover_anywhere_has_no_poster_rather_than_no_card() { + let mut d = pride(); + d.cover_i = None; + let r = to_record_with(&d, None, &q("", &[]), true).unwrap(); + assert!(!r.fields.contains_key("poster_url")); + } + + #[test] + fn isbn_cover_prefers_an_isbn13_and_ignores_junk() { + let mut d = SearchDoc::default(); + d.isbn = vec!["not-an-isbn".into(), "0306406152".into(), "9780306406157".into()]; + assert_eq!( + d.isbn_cover_url().as_deref(), + Some("https://covers.openlibrary.org/b/isbn/9780306406157-L.jpg?default=false") + ); + d.isbn = vec!["0306406152".into()]; + assert!(d.isbn_cover_url().unwrap().contains("0306406152")); + d.isbn = vec!["x".into()]; + assert_eq!(d.isbn_cover_url(), None); + } + + #[tokio::test] + async fn a_named_work_without_cover_or_description_still_gets_its_card_but_a_search_does_not() { + let server = MockServer::start().await; + let doc = json!({ + "key": "/works/OL39284292W", + "title": "Mother of Learning", + "author_name": ["nobody103"], + "first_publish_year": 2021, + "language": ["eng"], + "isbn": ["9781955252096"] + }); + // The lookup by id and the free-text search both find the same index doc. + Mock::given(method("GET")) + .and(path("/search.json")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"numFound": 1, "docs": [doc]}))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/works/OL39284292W.json")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "key": "/works/OL39284292W", "title": "Mother of Learning" + }))) + .mount(&server) + .await; + let (p, _dir) = plugin_against(&server).await; + + let by_id = p + .handle_query( + &q("", &[("openlibraryid", "OL39284292W"), ("contentKind", "book")]), + 10, + ) + .await + .unwrap(); + assert_eq!(by_id.len(), 1, "the work page lookup yields a card"); + assert_eq!(by_id[0].fields["title"], "Mother of Learning"); + assert!(by_id[0].fields["poster_url"].contains("/b/isbn/9781955252096-L.jpg")); + assert!(!by_id[0].fields.keys().any(|k| k.starts_with("description"))); + + let by_search = p + .handle_query(&q("mother of learning", &[("fileType", "card"), ("contentKind", "book")]), 10) + .await + .unwrap(); + assert!(by_search.is_empty(), "search rows keep the strict bar"); + } + #[test] fn echoes_browse_markers_but_no_other_filter() { let r = to_record( diff --git a/feeder-plugin/book-feeder/src/openlibrary_client.rs b/feeder-plugin/book-feeder/src/openlibrary_client.rs index fcd7239..2d13c49 100644 --- a/feeder-plugin/book-feeder/src/openlibrary_client.rs +++ b/feeder-plugin/book-feeder/src/openlibrary_client.rs @@ -30,6 +30,9 @@ pub const DEFAULT_API_BASE: &str = "https://openlibrary.org"; /// Cover image base. `-L` is the large rendition; the gateway seeds whatever /// `poster_url` names, so one size is published, not three. const COVERS_BASE: &str = "https://covers.openlibrary.org/b/id"; +/// Cover by ISBN. Open Library often holds an *edition's* cover for a work whose +/// own `covers` list is empty. +const COVERS_ISBN_BASE: &str = "https://covers.openlibrary.org/b/isbn"; /// Config key (and env seed) for the operator contact sent in the User-Agent. pub const CONTACT_KEY: &str = "contact"; @@ -58,6 +61,9 @@ const RETRY_BACKOFF_MS: u64 = 250; /// The field selection for every doc-shaped request, written once so a card /// resolved by search and the same card resolved by id are byte-identical. const SEARCH_FIELDS: &str = "key,title,author_name,cover_i,first_publish_year,language"; +/// A lookup of one named work also asks for its ISBNs, so a cover can be found by +/// ISBN. Not on the shared list: a search row would carry hundreds of ISBNs per doc. +const DOC_BY_ID_FIELDS: &str = "key,title,author_name,cover_i,first_publish_year,language,isbn"; /// Cache lifetimes. /// @@ -199,6 +205,10 @@ pub struct SearchDoc { pub first_publish_year: Option, #[serde(default)] pub id_project_gutenberg: Vec, + /// ISBNs of the work's editions. Only requested for an `openlibraryid` + /// lookup (see [`DOC_BY_ID_FIELDS`]); empty everywhere else. + #[serde(default)] + pub isbn: Vec, } impl SearchDoc { @@ -214,6 +224,22 @@ impl SearchDoc { .filter(|c| *c > 0) .map(|c| format!("{COVERS_BASE}/{c}-L.jpg")) } + + /// A cover looked up by one of the work's ISBNs (an ISBN-13 when there is one), + /// for a work with no cover of its own. + /// + /// `?default=false` makes Open Library answer 404 when it has no image for the + /// ISBN, instead of a 1×1 placeholder that would be seeded as the poster. + pub fn isbn_cover_url(&self) -> Option { + let digits_only = |s: &&String| s.chars().all(|c| c.is_ascii_digit()); + let isbn = self + .isbn + .iter() + .filter(digits_only) + .find(|s| s.len() == 13) + .or_else(|| self.isbn.iter().filter(digits_only).find(|s| s.len() == 10))?; + Some(format!("{COVERS_ISBN_BASE}/{isbn}-L.jpg?default=false")) + } } /// What a work JSON contributes to a card. @@ -440,12 +466,13 @@ impl OpenLibraryClient { ) -> Result, GatewayError> { let body = self .get_cached( - &format!("openlibrary:key:{id}"), + // `key2`: entries cached before ISBNs were requested lack them. + &format!("openlibrary:key2:{id}"), WORK_TTL_SECS, "/search.json", &[ ("q", format!("key:/works/{id}")), - ("fields", SEARCH_FIELDS.into()), + ("fields", DOC_BY_ID_FIELDS.into()), ("limit", "1".into()), ], deadline, @@ -493,6 +520,7 @@ impl OpenLibraryClient { .and_then(|a| a.iter().filter_map(Value::as_i64).find(|c| *c > 0)), first_publish_year: None, id_project_gutenberg: Vec::new(), + isbn: Vec::new(), })) }