Compare commits
1
Commits
main
...
fix/genre-rows
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2c7faff890 |
@@ -109,6 +109,62 @@ fn browse_kind(q: &GatewayQuery) -> Option<Browse> {
|
|||||||
/// The browse markers a card echoes. See [`to_record`].
|
/// The browse markers a card echoes. See [`to_record`].
|
||||||
const BROWSE_MARKERS: &[&str] = &["popular", "fresh", "trending"];
|
const BROWSE_MARKERS: &[&str] = &["popular", "fresh", "trending"];
|
||||||
|
|
||||||
|
/// One `genres:<slug>` value: the Open Library subject key it narrows to and
|
||||||
|
/// the name a card stores under `genres/`.
|
||||||
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
|
struct Genre {
|
||||||
|
subject: String,
|
||||||
|
name: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The genres a query asks for, in order; empty when it asks for none.
|
||||||
|
///
|
||||||
|
/// The wire carries a slug (`genres:science-fiction`, meta-read `config.rs`);
|
||||||
|
/// Open Library's subject key is the same words joined by `_`
|
||||||
|
/// (`science_fiction`), and the stored name is the words capitalised
|
||||||
|
/// (`Science Fiction`). The gateway's filter folds both sides to alphanumerics,
|
||||||
|
/// so the name matches the slug.
|
||||||
|
///
|
||||||
|
/// `None` when a value is not a plain slug (`[A-Za-z0-9_-]`): it would be
|
||||||
|
/// pasted into an Open Library query, and a row whose genre cannot be read
|
||||||
|
/// answers nothing rather than every genre.
|
||||||
|
fn requested_genres(q: &GatewayQuery) -> Option<Vec<Genre>> {
|
||||||
|
let mut out: Vec<Genre> = Vec::new();
|
||||||
|
for raw in q.filters.get("genres").into_iter().flatten() {
|
||||||
|
let raw = raw.trim();
|
||||||
|
if raw.is_empty() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if !raw.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let words: Vec<String> = raw
|
||||||
|
.split(['-', '_'])
|
||||||
|
.filter(|w| !w.is_empty())
|
||||||
|
.map(str::to_ascii_lowercase)
|
||||||
|
.collect();
|
||||||
|
if words.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let subject = words.join("_");
|
||||||
|
if out.iter().any(|g| g.subject == subject) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let name = words
|
||||||
|
.iter()
|
||||||
|
.map(|w| {
|
||||||
|
let mut c = w.chars();
|
||||||
|
c.next()
|
||||||
|
.map(|f| f.to_ascii_uppercase().to_string() + c.as_str())
|
||||||
|
.unwrap_or_default()
|
||||||
|
})
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(" ");
|
||||||
|
out.push(Genre { subject, name });
|
||||||
|
}
|
||||||
|
Some(out)
|
||||||
|
}
|
||||||
|
|
||||||
/// Identity and instalment filters this feeder cannot answer.
|
/// Identity and instalment filters this feeder cannot answer.
|
||||||
///
|
///
|
||||||
/// ⚠ An id-anchored query this upstream does not understand must return
|
/// ⚠ An id-anchored query this upstream does not understand must return
|
||||||
@@ -168,6 +224,11 @@ fn foreign_identity_filter(q: &GatewayQuery) -> Option<&str> {
|
|||||||
/// would stamp e.g. a `licence:pd` query onto a card — and the gateway stores
|
/// would stamp e.g. a `licence:pd` query onto a card — and the gateway stores
|
||||||
/// what passes, permanently. A card that lacks another filter's key does not
|
/// what passes, permanently. A card that lacks another filter's key does not
|
||||||
/// match it, which is the truth.
|
/// match it, which is the truth.
|
||||||
|
///
|
||||||
|
/// Genres are written from the doc, not echoed: a requested genre is stamped
|
||||||
|
/// only when the doc's `subject_key` lists it, which only a subject-narrowed
|
||||||
|
/// row fetches. The gateway's genre filter fails closed, so without the stamp
|
||||||
|
/// a genre row renders empty.
|
||||||
pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Option<DiscoveryRecord> {
|
pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Option<DiscoveryRecord> {
|
||||||
let id = doc.work_id()?;
|
let id = doc.work_id()?;
|
||||||
let cid = card_cid(id)?;
|
let cid = card_cid(id)?;
|
||||||
@@ -238,6 +299,12 @@ pub fn to_record(doc: &SearchDoc, description: &str, query: &GatewayQuery) -> Op
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for g in requested_genres(query).unwrap_or_default() {
|
||||||
|
if doc.subject_key.iter().any(|s| s.eq_ignore_ascii_case(&g.subject)) {
|
||||||
|
fields.insert(format!("genres/{}", g.name), "true".into());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
Some(DiscoveryRecord {
|
Some(DiscoveryRecord {
|
||||||
upstream_id: UPSTREAM.into(),
|
upstream_id: UPSTREAM.into(),
|
||||||
record_id: id.to_string(),
|
record_id: id.to_string(),
|
||||||
@@ -440,18 +507,27 @@ impl FeederPlugin for OpenLibraryPlugin {
|
|||||||
return Ok(project(client, vec![doc], query, 1, SEARCH_DEADLINE).await);
|
return Ok(project(client, vec![doc], query, 1, SEARCH_DEADLINE).await);
|
||||||
}
|
}
|
||||||
|
|
||||||
// A curated row.
|
// A curated row, narrowed to its genres when it names any.
|
||||||
if let Some(kind) = browse_kind(query) {
|
if let Some(kind) = browse_kind(query) {
|
||||||
|
let Some(genres) = requested_genres(query) else {
|
||||||
|
debug!("openlibrary: unreadable genres filter; returning nothing");
|
||||||
|
return Ok(Vec::new());
|
||||||
|
};
|
||||||
|
let subjects: Vec<String> = genres.into_iter().map(|g| g.subject).collect();
|
||||||
let n = self.discovery_n.min(cap);
|
let n = self.discovery_n.min(cap);
|
||||||
let docs = match kind {
|
let docs = match kind {
|
||||||
Browse::Popular => {
|
Browse::Popular => {
|
||||||
client
|
client
|
||||||
.trending((n * SEARCH_OVERFETCH).min(MAX_LIMIT), BROWSE_DEADLINE)
|
.trending(
|
||||||
|
(n * SEARCH_OVERFETCH).min(MAX_LIMIT),
|
||||||
|
&subjects,
|
||||||
|
BROWSE_DEADLINE,
|
||||||
|
)
|
||||||
.await?
|
.await?
|
||||||
}
|
}
|
||||||
Browse::Fresh => {
|
Browse::Fresh => {
|
||||||
client
|
client
|
||||||
.fresh((n * FRESH_OVERFETCH).min(MAX_LIMIT), BROWSE_DEADLINE)
|
.fresh((n * FRESH_OVERFETCH).min(MAX_LIMIT), &subjects, BROWSE_DEADLINE)
|
||||||
.await?
|
.await?
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -545,6 +621,7 @@ mod tests {
|
|||||||
cover_i: Some(14348537),
|
cover_i: Some(14348537),
|
||||||
first_publish_year: Some(1813),
|
first_publish_year: Some(1813),
|
||||||
id_project_gutenberg: vec![],
|
id_project_gutenberg: vec![],
|
||||||
|
subject_key: vec![],
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -974,6 +1051,109 @@ mod tests {
|
|||||||
server.verify().await;
|
server.verify().await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn genre(subject: &str, name: &str) -> Genre {
|
||||||
|
Genre { subject: subject.into(), name: name.into() }
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn genre_slugs_map_to_open_library_subjects_and_card_names() {
|
||||||
|
assert_eq!(
|
||||||
|
requested_genres(&q("", &[("genres", "science-fiction")])),
|
||||||
|
Some(vec![genre("science_fiction", "Science Fiction")])
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requested_genres(&q("", &[("genres", "Fantasy")])),
|
||||||
|
Some(vec![genre("fantasy", "Fantasy")])
|
||||||
|
);
|
||||||
|
let mut several = q("", &[]);
|
||||||
|
several.filters.insert(
|
||||||
|
"genres".into(),
|
||||||
|
vec!["fantasy".into(), "Fantasy".into(), "mystery".into()],
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
requested_genres(&several),
|
||||||
|
Some(vec![genre("fantasy", "Fantasy"), genre("mystery", "Mystery")])
|
||||||
|
);
|
||||||
|
assert_eq!(requested_genres(&q("", &[])), Some(vec![]));
|
||||||
|
// The subject is pasted into an Open Library query: anything but a
|
||||||
|
// plain slug is refused.
|
||||||
|
assert_eq!(requested_genres(&q("", &[("genres", "fantasy)OR(*")])), None);
|
||||||
|
assert_eq!(requested_genres(&q("", &[("genres", "--")])), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_card_names_only_the_requested_genres_open_library_files_it_under() {
|
||||||
|
let mut doc = pride();
|
||||||
|
doc.subject_key = vec!["fantasy".into(), "magic".into()];
|
||||||
|
|
||||||
|
let row = q("", &[("popular", "true"), ("genres", "fantasy")]);
|
||||||
|
let r = to_record(&doc, "A novel.", &row).unwrap();
|
||||||
|
assert_eq!(r.fields["genres/Fantasy"], "true");
|
||||||
|
assert_eq!(r.fields.keys().filter(|k| k.starts_with("genres/")).count(), 1);
|
||||||
|
assert!(meta_feeder_sdk::query_eval::record_matches(&r.fields, &row));
|
||||||
|
|
||||||
|
let other = q("", &[("popular", "true"), ("genres", "mystery")]);
|
||||||
|
let r = to_record(&doc, "A novel.", &other).unwrap();
|
||||||
|
assert!(!r.fields.keys().any(|k| k.starts_with("genres")));
|
||||||
|
assert!(!meta_feeder_sdk::query_eval::record_matches(&r.fields, &other));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_genre_row_reads_trending_search_narrowed_to_its_subject() {
|
||||||
|
let server = MockServer::start().await;
|
||||||
|
let mut dune = doc_json("OL893415W", "Dune", Some(11481354));
|
||||||
|
dune["subject_key"] = json!(["science_fiction", "fiction"]);
|
||||||
|
Mock::given(method("GET"))
|
||||||
|
.and(path("/search.json"))
|
||||||
|
.and(query_param("q", "subject_key:(science_fiction)"))
|
||||||
|
.and(query_param("sort", "trending"))
|
||||||
|
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||||
|
"numFound": 1,
|
||||||
|
"docs": [dune]
|
||||||
|
})))
|
||||||
|
.expect(1)
|
||||||
|
.mount(&server)
|
||||||
|
.await;
|
||||||
|
Mock::given(method("GET"))
|
||||||
|
.and(path("/trending/daily.json"))
|
||||||
|
.respond_with(ResponseTemplate::new(500))
|
||||||
|
.expect(0)
|
||||||
|
.mount(&server)
|
||||||
|
.await;
|
||||||
|
Mock::given(method("GET"))
|
||||||
|
.and(path("/works/OL893415W.json"))
|
||||||
|
.respond_with(ResponseTemplate::new(200).set_body_json(json!({ "key": "/works/OL893415W", "description": "A novel." })))
|
||||||
|
.mount(&server)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let (p, _dir) = plugin_against(&server).await;
|
||||||
|
// meta-read's Science Fiction row, as `config.rs` spells it.
|
||||||
|
let row = q(
|
||||||
|
"",
|
||||||
|
&[
|
||||||
|
("popular", "true"),
|
||||||
|
("domain", "literature"),
|
||||||
|
("contentKind", "book"),
|
||||||
|
("fileType", "card"),
|
||||||
|
("genres", "science-fiction"),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
let out = p.handle_query(&row, 20).await.unwrap();
|
||||||
|
assert_eq!(out.len(), 1);
|
||||||
|
assert_eq!(out[0].fields["genres/Science Fiction"], "true");
|
||||||
|
// The gateway's filter fails closed on genres; it keeps this card.
|
||||||
|
assert!(meta_feeder_sdk::query_eval::record_matches(&out[0].fields, &row));
|
||||||
|
server.verify().await;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn an_unreadable_genre_answers_nothing_before_any_request() {
|
||||||
|
let (p, server, _dir) = plugin_that_must_not_call_out().await;
|
||||||
|
let row = q("", &[("popular", "true"), ("genres", "fantasy)OR(*")]);
|
||||||
|
assert!(p.handle_query(&row, 20).await.unwrap().is_empty());
|
||||||
|
server.verify().await;
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn a_rate_limited_search_surfaces_as_rate_limited() {
|
async fn a_rate_limited_search_surfaces_as_rate_limited() {
|
||||||
let server = MockServer::start().await;
|
let server = MockServer::start().await;
|
||||||
|
|||||||
@@ -59,6 +59,12 @@ const RETRY_BACKOFF_MS: u64 = 250;
|
|||||||
/// resolved by search and the same card resolved by id are byte-identical.
|
/// 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";
|
const SEARCH_FIELDS: &str = "key,title,author_name,cover_i,first_publish_year,language";
|
||||||
|
|
||||||
|
/// [`SEARCH_FIELDS`] plus the work's subject keys, read only by a row narrowed
|
||||||
|
/// to subjects: a card names a genre only when Open Library files the work
|
||||||
|
/// under it.
|
||||||
|
const SUBJECT_SEARCH_FIELDS: &str =
|
||||||
|
"key,title,author_name,cover_i,first_publish_year,language,subject_key";
|
||||||
|
|
||||||
/// Cache lifetimes.
|
/// Cache lifetimes.
|
||||||
///
|
///
|
||||||
/// ⚠ Unlike AniList's, these lists are not immutable. `trending/daily` changes
|
/// ⚠ Unlike AniList's, these lists are not immutable. `trending/daily` changes
|
||||||
@@ -199,6 +205,10 @@ pub struct SearchDoc {
|
|||||||
pub first_publish_year: Option<i32>,
|
pub first_publish_year: Option<i32>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub id_project_gutenberg: Vec<String>,
|
pub id_project_gutenberg: Vec<String>,
|
||||||
|
/// Only requested by a subject-narrowed row ([`SUBJECT_SEARCH_FIELDS`]);
|
||||||
|
/// empty everywhere else.
|
||||||
|
#[serde(default)]
|
||||||
|
pub subject_key: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl SearchDoc {
|
impl SearchDoc {
|
||||||
@@ -378,11 +388,33 @@ impl OpenLibraryClient {
|
|||||||
/// The `popular:true` row: what Open Library readers opened today.
|
/// The `popular:true` row: what Open Library readers opened today.
|
||||||
///
|
///
|
||||||
/// The ordering is a popularity signal, but no count is read or stored.
|
/// The ordering is a popularity signal, but no count is read or stored.
|
||||||
|
///
|
||||||
|
/// `subjects` narrows the row to works filed under any of them (a genre
|
||||||
|
/// row). `/trending/daily.json` takes no subject, so a narrowed row asks
|
||||||
|
/// `search.json` sorted `trending`, the same signal over a filtered set.
|
||||||
pub async fn trending(
|
pub async fn trending(
|
||||||
&self,
|
&self,
|
||||||
limit: usize,
|
limit: usize,
|
||||||
|
subjects: &[String],
|
||||||
deadline: Duration,
|
deadline: Duration,
|
||||||
) -> Result<Vec<SearchDoc>, GatewayError> {
|
) -> Result<Vec<SearchDoc>, GatewayError> {
|
||||||
|
if !subjects.is_empty() {
|
||||||
|
let body = self
|
||||||
|
.get_cached(
|
||||||
|
&format!("openlibrary:trending:subjects:{}:{limit}", subjects.join("+")),
|
||||||
|
BROWSE_TTL_SECS,
|
||||||
|
"/search.json",
|
||||||
|
&[
|
||||||
|
("q", subject_clause(subjects)),
|
||||||
|
("sort", "trending".into()),
|
||||||
|
("fields", SUBJECT_SEARCH_FIELDS.into()),
|
||||||
|
("limit", limit.to_string()),
|
||||||
|
],
|
||||||
|
deadline,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
return Ok(parse_docs(body.get("docs")));
|
||||||
|
}
|
||||||
let body = self
|
let body = self
|
||||||
.get_cached(
|
.get_cached(
|
||||||
&format!("openlibrary:trending:daily:{limit}"),
|
&format!("openlibrary:trending:daily:{limit}"),
|
||||||
@@ -402,21 +434,30 @@ impl OpenLibraryClient {
|
|||||||
/// over a broad query is dominated by coverless catalogue stubs. The year
|
/// over a broad query is dominated by coverless catalogue stubs. The year
|
||||||
/// window keeps the row about new books; the card gate (cover + description)
|
/// window keeps the row about new books; the card gate (cover + description)
|
||||||
/// drops the stubs that remain, so the caller over-fetches.
|
/// drops the stubs that remain, so the caller over-fetches.
|
||||||
|
///
|
||||||
|
/// `subjects` narrows the row as in [`Self::trending`].
|
||||||
pub async fn fresh(
|
pub async fn fresh(
|
||||||
&self,
|
&self,
|
||||||
limit: usize,
|
limit: usize,
|
||||||
|
subjects: &[String],
|
||||||
deadline: Duration,
|
deadline: Duration,
|
||||||
) -> Result<Vec<SearchDoc>, GatewayError> {
|
) -> Result<Vec<SearchDoc>, GatewayError> {
|
||||||
let year = 1970 + now_secs() / 31_556_952;
|
let year = 1970 + now_secs() / 31_556_952;
|
||||||
|
let mut q = format!("first_publish_year:[{} TO {year}]", year - 1);
|
||||||
|
let mut fields = SEARCH_FIELDS;
|
||||||
|
if !subjects.is_empty() {
|
||||||
|
q = format!("{q} AND {}", subject_clause(subjects));
|
||||||
|
fields = SUBJECT_SEARCH_FIELDS;
|
||||||
|
}
|
||||||
let body = self
|
let body = self
|
||||||
.get_cached(
|
.get_cached(
|
||||||
&format!("openlibrary:fresh:{year}:{limit}"),
|
&format!("openlibrary:fresh:{year}:{}:{limit}", subjects.join("+")),
|
||||||
BROWSE_TTL_SECS,
|
BROWSE_TTL_SECS,
|
||||||
"/search.json",
|
"/search.json",
|
||||||
&[
|
&[
|
||||||
("q", format!("first_publish_year:[{} TO {year}]", year - 1)),
|
("q", q),
|
||||||
("sort", "new".into()),
|
("sort", "new".into()),
|
||||||
("fields", SEARCH_FIELDS.into()),
|
("fields", fields.into()),
|
||||||
("limit", limit.to_string()),
|
("limit", limit.to_string()),
|
||||||
],
|
],
|
||||||
deadline,
|
deadline,
|
||||||
@@ -493,6 +534,7 @@ impl OpenLibraryClient {
|
|||||||
.and_then(|a| a.iter().filter_map(Value::as_i64).find(|c| *c > 0)),
|
.and_then(|a| a.iter().filter_map(Value::as_i64).find(|c| *c > 0)),
|
||||||
first_publish_year: None,
|
first_publish_year: None,
|
||||||
id_project_gutenberg: Vec::new(),
|
id_project_gutenberg: Vec::new(),
|
||||||
|
subject_key: Vec::new(),
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -880,6 +922,12 @@ fn single_gutenberg_work(body: &Value, gutenberg_id: u64) -> Option<String> {
|
|||||||
doc.work_id().map(str::to_string)
|
doc.work_id().map(str::to_string)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `subject_key:(a OR b)`. The keys come from [`crate::openlibrary`]'s genre
|
||||||
|
/// parser, which admits only `[a-z0-9_]`, so nothing here needs escaping.
|
||||||
|
fn subject_clause(subjects: &[String]) -> String {
|
||||||
|
format!("subject_key:({})", subjects.join(" OR "))
|
||||||
|
}
|
||||||
|
|
||||||
fn parse_docs(v: Option<&Value>) -> Vec<SearchDoc> {
|
fn parse_docs(v: Option<&Value>) -> Vec<SearchDoc> {
|
||||||
v.and_then(Value::as_array)
|
v.and_then(Value::as_array)
|
||||||
.map(|a| {
|
.map(|a| {
|
||||||
@@ -1004,6 +1052,15 @@ mod tests {
|
|||||||
assert_eq!(clean_description(" \r\n "), None);
|
assert_eq!(clean_description(" \r\n "), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn subject_clause_ors_the_keys() {
|
||||||
|
assert_eq!(subject_clause(&["fantasy".to_string()]), "subject_key:(fantasy)");
|
||||||
|
assert_eq!(
|
||||||
|
subject_clause(&["fantasy".to_string(), "mystery".to_string()]),
|
||||||
|
"subject_key:(fantasy OR mystery)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn cover_minus_one_is_no_cover() {
|
fn cover_minus_one_is_no_cover() {
|
||||||
let mut d = SearchDoc {
|
let mut d = SearchDoc {
|
||||||
|
|||||||
Reference in New Issue
Block a user