Merge remote-tracking branch 'origin/main' into bibfetch-openlibrary-cover

Review id 3 on catena#2.

P0: Record.cover_id came from the search index's cover_i, the cover
chosen for the whole work, and the index is asked first, so cover()
fell back to another edition's image (Penguin War and Peace,
9780140447934, got Standard Ebooks' Maude cover 12621906). cover_i is
no longer asked for or read; cover_id is the first positive id in the
edition record's covers, or None. An ISBN request that fails, as
opposed to a 404, now ends the fetch with that failure instead of
going on to the cover id.

Redirects go only over HTTPS on the standard port to
covers.openlibrary.org, archive.org or a host under archive.org, and
never to an address by number; the covers store the fetch began at is
still allowed.

The 1 KB size floor is gone. With default=false a missing cover is a
404, and the stand-in without it is a 43-byte GIF the media type check
refuses, so the floor could only drop real Small covers.

Covers.deadline (DEADLINE, two minutes) bounds a whole fetch: every
address, attempt, redirect, body and wait. It starts once the fetch
holds the process lock. The timeout test no longer times a span that
includes the lock wait.

crates/bibfetch/README.md

@@ -107,6 +107,47 @@ register that is rate-limiting, short enough that one which is simply down
 does not hold up a pipeline. An answer this cannot read is not retried,
 because it will not read any better next time.
 
+## Covers
+
+```sh
+bibfetch cover 978-0-306-40615-7 -o cover.jpg
+```
+
+Fetches a book's cover from Open Library's covers store by its ISBN and
+writes the image. The thirteen-digit form is asked first and the ten-digit
+form second, because a book is often filed under only one of them; an ISBN
+whose check digit is wrong is asked about as written, since converting it
+would give another book's number. `--cover-id` names an Open Library cover id
+to ask by last, which is what the `cover_id` field of a lookup record holds:
+the first cover in Open Library's record of that edition, never the cover its
+index picked for the whole work, which is often another edition's. The cover
+id is asked only when every form of the ISBN asked was answered with no cover; if
+asking by ISBN failed, that failure is the result.
+
+What comes back is checked before it is written: a JPEG or PNG by its bytes
+as well as by what the server says, no larger than ten megabytes, reached in at
+most three redirects, each over HTTPS to `covers.openlibrary.org`,
+`archive.org` or a host under `archive.org`, and never to an address by number.
+One request is made at a time, a `429` or `503` is waited out for as long as
+its `Retry-After` says, up to a minute, and the whole fetch, every attempt and
+redirect included, stops after two minutes. The two minutes start once it is
+the fetch's turn, so a fetch queued behind others in the same process can wait
+that long again first.
+
+| Option | Effect |
+|---|---|
+| `-o`, `--output <FILE>` | Where to write the image |
+| `--size <SIZE>` | `small`, `medium` or `large`. Default `large` |
+| `--cover-id <ID>` | An Open Library cover id to ask by when the ISBN has none |
+
+Exit `0` the cover was written, `1` Open Library has none, `65` the argument
+was not an ISBN or the answer was not an image, `69` Open Library could not be
+reached, `74` the file could not be written.
+
+From Rust, `bibfetch::cover(&agent, isbn, cover_id, size)` returns
+`Ok(None)` when there is no cover, and a [`Cover`](src/cover.rs) with the
+bytes, the media type and the address asked when there is.
+
 ## Reaching the network
 
 This and [`emb`](../emb/README.md) are the only two tools here that do. Both
@@ -114,8 +155,9 @@ say so in `--help`, and for both it is the whole of what they do rather than
 something that happens on the way to something else — which is the condition
 under which [the contract](../../docs/contract.md) allows it at all.
 
-Nothing is cached, nothing is stored, and nothing is sent but the identifier
-and the address you pass to `--polite`.
+Nothing is cached, nothing is stored but a cover you asked to have written,
+and nothing is sent but the identifier and the address you pass to
+`--polite`.
 
 ## See also
 

crates/bibfetch/src/lib.rs

@@ -14,6 +14,7 @@
 //! ISSN is looked up as the journal of a work rather than on its own.
 
 pub mod ask;
+pub mod cover;
 pub mod source;
 
 use std::time::Duration;
@@ -21,6 +22,7 @@ use std::time::Duration;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
+pub use cover::{Cover, CoverSize, cover};
 pub use source::Source;
 
 /// How long to wait on a register before giving up.
@@ -74,6 +76,13 @@ pub struct Record {
     /// How many pages the register says it has.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub pages: Option<u32>,
+    /// The first cover id in Open Library's record of this edition, for
+    /// [`cover()`] to ask by when the ISBN has none. Taken from the edition
+    /// record only, never from the index's cover for the work, which is
+    /// often another edition's; `None` when the edition names no cover or
+    /// the lookup stopped before asking for the edition record.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub cover_id: Option<u64>,
 }
 
 fn version() -> u32 {
@@ -95,6 +104,8 @@ pub enum Error {
     Shape(String),
     /// Nothing here knows where to ask about this kind.
     NoRegister(String),
+    /// What was asked about is not an identifier of the kind it should be.
+    Invalid(String),
 }
 
 impl std::fmt::Display for Error {
@@ -105,6 +116,7 @@ impl std::fmt::Display for Error {
             Error::NoRegister(kind) => {
                 write!(f, "no register here answers about {kind} identifiers")
             }
+            Error::Invalid(why) => write!(f, "{why}"),
         }
     }
 }
@@ -130,6 +142,7 @@ pub fn missing(id: &str, kind: &str, source: Source) -> Record {
         subjects: Vec::new(),
         language: None,
         pages: None,
+        cover_id: None,
     }
 }
 
@@ -187,6 +200,9 @@ impl Record {
         if self.pages.is_none() {
             self.pages = other.pages;
         }
+        if self.cover_id.is_none() {
+            self.cover_id = other.cover_id;
+        }
         // Which register answered becomes which registers did.
         if !self.source.split('+').any(|s| s == other.source) {
             self.source = format!("{}+{}", self.source, other.source);

crates/bibfetch/src/main.rs

@@ -29,33 +29,83 @@ Exit codes: 0 every identifier was found, 1 at least one was not,
 Example:
 
     idnorm refs.txt | bibfetch | jq -r 'select(.found) | .title'
-    idnorm --kind doi refs.txt | bibfetch --polite [email protected]"
+    idnorm --kind doi refs.txt | bibfetch --polite [email protected]
+    bibfetch cover 978-0-306-40615-7 -o cover.jpg"
 )]
 struct Args {
+    #[command(subcommand)]
+    command: Option<Command>,
+
     /// Identifier records to look up, as `idnorm` writes them. Reads stdin
     /// when absent.
     path: Option<PathBuf>,
 
     /// An address to put in the request, which the registers ask for and
     /// answer faster when they get.
-    #[arg(long, value_name = "EMAIL")]
+    #[arg(long, global = true, value_name = "EMAIL")]
     polite: Option<String>,
 
     /// How long to wait on a register, in seconds.
-    #[arg(long, default_value_t = bibfetch::TIMEOUT.as_secs(), value_name = "SECONDS")]
+    #[arg(long, global = true, default_value_t = bibfetch::TIMEOUT.as_secs(), value_name = "SECONDS")]
     timeout: u64,
 
     /// How many times to ask before believing a failure.
-    #[arg(long, default_value_t = bibfetch::ATTEMPTS, value_name = "COUNT")]
+    #[arg(long, global = true, default_value_t = bibfetch::ATTEMPTS, value_name = "COUNT")]
     attempts: u32,
 }
 
+#[derive(clap::Subcommand)]
+enum Command {
+    /// Fetch a book's cover from Open Library by its ISBN.
+    ///
+    /// Exit codes: 0 the cover was written, 1 Open Library has none,
+    /// 65 not an ISBN or not an image, 69 Open Library could not be reached,
+    /// 74 the file could not be written.
+    Cover {
+        /// The ISBN, ten or thirteen digits, hyphens or not.
+        isbn: String,
+        /// Where to write the image.
+        #[arg(short, long, value_name = "FILE")]
+        output: PathBuf,
+        /// Which size to ask for.
+        #[arg(long, value_enum, default_value_t = Size::Large)]
+        size: Size,
+        /// An Open Library cover id to ask by when the ISBN has no cover.
+        #[arg(long, value_name = "ID")]
+        cover_id: Option<u64>,
+    },
+}
+
+#[derive(Clone, Copy, clap::ValueEnum)]
+enum Size {
+    Small,
+    Medium,
+    Large,
+}
+
 fn main() -> ExitCode {
     let args: Args = match catena_cli::parse() {
         Ok(args) => args,
         Err(code) => return code,
     };
 
+    let agent = bibfetch::ask::Ask {
+        polite: args.polite.clone(),
+        timeout: Duration::from_secs(args.timeout),
+        attempts: args.attempts,
+    }
+    .agent();
+
+    if let Some(Command::Cover {
+        isbn,
+        output,
+        size,
+        cover_id,
+    }) = &args.command
+    {
+        return cover(&agent, args.attempts, isbn, output, *size, *cover_id);
+    }
+
     let text = match catena_cli::read_text(args.path.as_deref()) {
         Ok(text) => text,
         Err((code, why)) => {
@@ -64,13 +114,6 @@ fn main() -> ExitCode {
         }
     };
 
-    let agent = bibfetch::ask::Ask {
-        polite: args.polite.clone(),
-        timeout: Duration::from_secs(args.timeout),
-        attempts: args.attempts,
-    }
-    .agent();
-
     let mut records = Vec::new();
     let mut missing = 0usize;
 
@@ -114,6 +157,46 @@ fn main() -> ExitCode {
     })
 }
 
+/// Fetch one cover and write it where asked.
+fn cover(
+    agent: &ureq::Agent,
+    attempts: u32,
+    isbn: &str,
+    output: &std::path::Path,
+    size: Size,
+    cover_id: Option<u64>,
+) -> ExitCode {
+    let covers = bibfetch::cover::Covers {
+        attempts,
+        ..Default::default()
+    };
+    let size = match size {
+        Size::Small => bibfetch::CoverSize::Small,
+        Size::Medium => bibfetch::CoverSize::Medium,
+        Size::Large => bibfetch::CoverSize::Large,
+    };
+    match covers.fetch(agent, isbn, cover_id, size) {
+        Ok(Some(found)) => match std::fs::write(output, &found.bytes) {
+            Ok(()) => ExitCode::from(exit::OK),
+            Err(why) => {
+                eprintln!("bibfetch: {}: {why}", output.display());
+                ExitCode::from(exit::IO)
+            }
+        },
+        Ok(None) => {
+            eprintln!("bibfetch: {isbn}: Open Library has no cover");
+            ExitCode::from(exit::NO)
+        }
+        Err(why) => {
+            eprintln!("bibfetch: {isbn}: {why}");
+            ExitCode::from(match why {
+                bibfetch::Error::Unreachable(_) => exit::UNAVAILABLE,
+                _ => exit::DATA,
+            })
+        }
+    }
+}
+
 /// The identifier and its kind out of one record.
 fn read_identifier(line: &str) -> Result<(String, String), String> {
     let value: serde_json::Value =

crates/bibfetch/src/source.rs

@@ -85,6 +85,9 @@ impl Source {
             }
             // `fields` or the index answers with a handful of defaults and
             // keeps the rest, which is how a language it holds went unread.
+            // Not `cover_i`: that is the cover chosen for the whole work, and
+            // on a work with hundreds of editions it is usually another
+            // edition's, in another translation or language.
             Source::OpenLibrarySearch => format!(
                 "https://openlibrary.org/search.json?q=isbn:{}\
 &fields=title,subtitle,author_name,first_publish_year,publisher,language,subject,\
@@ -245,6 +248,9 @@ struct OpenLibraryBook {
     languages: Vec<OpenLibraryKey>,
     #[serde(default)]
     number_of_pages: Option<u32>,
+    /// Cover ids, best first. A deleted cover is left in the list as `-1`.
+    #[serde(default)]
+    covers: Vec<i64>,
 }
 
 #[derive(serde::Deserialize)]
@@ -275,6 +281,11 @@ fn open_library(id: &str, kind: &str, body: &str) -> Result<Record, Error> {
         .find_map(|l| l.key.rsplit('/').next().map(str::to_string))
         .filter(|l| !l.is_empty());
     record.pages = book.number_of_pages;
+    // The edition's own cover, and the only place a cover id is taken from.
+    record.cover_id = book
+        .covers
+        .into_iter()
+        .find_map(|id| u64::try_from(id).ok().filter(|id| *id > 0));
     Ok(record)
 }
 
@@ -520,6 +531,56 @@ mod tests {
         }
     }
 
+    #[test]
+    fn the_cover_id_a_record_names_is_kept_for_the_cover_lookup() {
+        // A deleted cover stays in an edition's list as -1, ahead of the
+        // ones still there.
+        let record = open_library("1", "isbn", r#"{"title":"A","covers":[-1,1162784,99]}"#)
+            .expect("a record");
+        assert_eq!(record.cover_id, Some(1162784));
+        let record = open_library("1", "isbn", r#"{"title":"A"}"#).expect("a record");
+        assert_eq!(record.cover_id, None);
+    }
+
+    #[test]
+    fn the_cover_id_is_the_editions_and_not_the_works() {
+        // ISBN 9780140447934 today: the index (asked first) names the work's
+        // cover; the edition record has none. Merged as `ask::lookup` merges.
+        let mut kept = open_library_search(
+            "9780140447934",
+            "isbn",
+            r#"{"docs":[{"title":"War and Peace","cover_i":12621906}]}"#,
+        )
+        .expect("a record");
+        let edition = open_library("9780140447934", "isbn", r#"{"title":"War and Peace"}"#)
+            .expect("a record");
+        kept.fill_from(edition);
+        // Cover 12621906 is OL34941467M, Standard Ebooks' Maude translation.
+        assert_eq!(kept.cover_id, None);
+    }
+
+    #[test]
+    fn an_edition_with_no_covers_gives_no_cover_id() {
+        let index = r#"{"docs":[{"title":"War and Peace","cover_i":12621906}]}"#;
+        // `lookup` often stops at the index, before the edition is asked.
+        let alone = open_library_search("9780140447934", "isbn", index).expect("a record");
+        assert_eq!(alone.cover_id, None);
+        // An edition whose only cover was deleted, and one with an empty list.
+        for edition in [
+            r#"{"title":"War and Peace","covers":[-1]}"#,
+            r#"{"title":"War and Peace","covers":[]}"#,
+        ] {
+            let mut kept = open_library_search("9780140447934", "isbn", index).expect("a record");
+            kept.fill_from(open_library("9780140447934", "isbn", edition).expect("a record"));
+            assert_eq!(kept.cover_id, None, "{edition}");
+        }
+    }
+
+    #[test]
+    fn the_index_is_not_asked_for_the_works_cover() {
+        assert!(!Source::OpenLibrarySearch.url("1").contains("cover_i"));
+    }
+
     #[test]
     fn an_answer_that_is_not_what_it_should_be_says_so() {
         assert!(matches!(

crates/idnorm/src/isbn.rs

@@ -0,0 +1,99 @@
+//! An ISBN's two forms, ten digits and thirteen.
+//!
+//! A book printed before 2007 carries a ten-digit ISBN, one printed since
+//! carries thirteen, and every ten-digit one has a thirteen-digit twin: `978`
+//! in front and the check digit worked out again. A catalogue may file a
+//! book under either, so asking about one form and not the other misses
+//! books that are there.
+//!
+//! Only a well-formed ISBN is converted. Working out a fresh check digit for
+//! one whose own check digit is wrong would not correct the typing error, it
+//! would turn the number into a different book's.
+
+/// Just the digits of an ISBN, with a check character of ten as `X`.
+pub fn digits(text: &str) -> String {
+    bibclean::isbn::digits(text).to_ascii_uppercase()
+}
+
+/// The thirteen-digit form of an ISBN, from either form.
+///
+/// `None` when the text is not an ISBN whose check digit agrees.
+pub fn to_13(text: &str) -> Option<String> {
+    let digits = digits(text);
+    if !bibclean::isbn::valid(&digits) {
+        return None;
+    }
+    match digits.len() {
+        13 => Some(digits),
+        10 => {
+            let body = format!("978{}", &digits[..9]);
+            let sum: u32 = body
+                .bytes()
+                .enumerate()
+                .map(|(at, b)| u32::from(b - b'0') * if at % 2 == 0 { 1 } else { 3 })
+                .sum();
+            Some(format!("{body}{}", (10 - sum % 10) % 10))
+        }
+        _ => None,
+    }
+}
+
+/// The ten-digit form of an ISBN, from either form.
+///
+/// `None` when the text is not an ISBN whose check digit agrees, or when it
+/// is a thirteen-digit one starting `979`, which was never issued in ten.
+pub fn to_10(text: &str) -> Option<String> {
+    let digits = digits(text);
+    if !bibclean::isbn::valid(&digits) {
+        return None;
+    }
+    match digits.len() {
+        10 => Some(digits),
+        13 if digits.starts_with("978") => {
+            let body = &digits[3..12];
+            let sum: u32 = body
+                .bytes()
+                .enumerate()
+                .map(|(at, b)| u32::from(b - b'0') * (10 - at as u32))
+                .sum();
+            let check = match (11 - sum % 11) % 11 {
+                10 => 'X',
+                n => char::from(b'0' + n as u8),
+            };
+            Some(format!("{body}{check}"))
+        }
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn each_form_becomes_the_other() {
+        assert_eq!(to_13("0-306-40615-2").as_deref(), Some("9780306406157"));
+        assert_eq!(to_10("978-0-306-40615-7").as_deref(), Some("0306406152"));
+        assert_eq!(to_13("9780306406157").as_deref(), Some("9780306406157"));
+        assert_eq!(to_10("0306406152").as_deref(), Some("0306406152"));
+    }
+
+    #[test]
+    fn a_check_digit_of_ten_is_written_x() {
+        assert_eq!(to_10("9780804429573").as_deref(), Some("080442957X"));
+        assert_eq!(to_13("080442957x").as_deref(), Some("9780804429573"));
+    }
+
+    #[test]
+    fn a_979_isbn_has_no_ten_digit_form() {
+        assert_eq!(to_10("979-10-90636-07-1"), None);
+        assert_eq!(to_13("979-10-90636-07-1").as_deref(), Some("9791090636071"));
+    }
+
+    #[test]
+    fn a_mistyped_isbn_is_not_converted_into_a_different_book() {
+        assert_eq!(to_13("0-306-40615-3"), None);
+        assert_eq!(to_10("978-0-306-40615-8"), None);
+        assert_eq!(to_13("12345"), None);
+    }
+}

crates/idnorm/src/lib.rs

@@ -13,6 +13,7 @@
 
 pub mod arxiv;
 pub mod doi;
+pub mod isbn;
 pub mod issn;
 
 use schemars::JsonSchema;