fix: keep the identifiers an EPUB declares

DC:identifier was validated verbatim, so hyphenated and urn:isbn: forms never reached the checksum and every non-ISBN identifier was discarded. Normalise first, and name whatever survives.
2026-08-15 21:54:12 -04:00
parent 86e1d096ef
commit 523117ec28
2 changed files with 127 additions and 6 deletions
@@ -31,6 +31,76 @@ from chitai.services.utils import (
 logger = logging.getLogger(__name__)


+# Identifier schemes an EPUB can declare, mapped onto the names the rest of the app
+# uses. A scheme arrives either as an `opf:scheme` attribute or as a prefix on the
+# value itself (`urn:isbn:…`, `calibre:…`), and the two say the same thing.
+_IDENTIFIER_SCHEMES = {
+    "isbn": "isbn",
+    "isbn10": "isbn-10",
+    "isbn-10": "isbn-10",
+    "isbn13": "isbn-13",
+    "isbn-13": "isbn-13",
+    "uuid": "uuid",
+    "calibre": "calibre",
+    "doi": "doi",
+    "asin": "asin",
+    "amazon": "asin",
+    "mobi-asin": "asin",
+    "google": "google",
+    "goodreads": "goodreads",
+}
+
+# `scheme:rest`, with an optional `urn:` in front of it.
+_SCHEME_PREFIX = re.compile(r"^(?:urn:)?([A-Za-z][A-Za-z0-9.-]*):(.+)$")
+
+_UUID = re.compile(r"^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$", re.IGNORECASE)
+
+
+def parse_identifier(value: str, scheme: str | None = None) -> tuple[str, str] | None:
+    """
+    Work out what one raw identifier is, and what it is worth storing as.
+
+    EPUBs write the same ISBN as `9780486282114`, `978-0-486-28211-4` and
+    `urn:isbn:978-0-486-28211-4`, and carry plenty of identifiers that are not ISBNs
+    at all. Validating the string verbatim keeps only the first form and throws the
+    rest away, so normalise first and name whatever survives.
+
+    Args:
+        value: The identifier as the file wrote it.
+        scheme: What the file said it is, if it said anything — an `opf:scheme`
+            attribute. A prefix on the value takes precedence over this.
+
+    Returns:
+        The `(name, value)` to store, or `None` when there is nothing usable: an
+        empty value, or one declared to be an ISBN that fails its own checksum.
+    """
+    value = (value or "").strip()
+    if not value:
+        return None
+
+    name = _IDENTIFIER_SCHEMES.get((scheme or "").strip().casefold())
+
+    # An unrecognised prefix is part of the value rather than a scheme —
+    # "http://example.com/book" is not an identifier called "http".
+    if (match := _SCHEME_PREFIX.match(value)) and (
+        prefixed := _IDENTIFIER_SCHEMES.get(match.group(1).casefold())
+    ):
+        name = prefixed
+        value = match.group(2).strip()
+
+    if name is None or name.startswith("isbn"):
+        digits = re.sub(r"[^0-9Xx]", "", value).upper()
+        if is_valid_isbn(digits):
+            return f"isbn-{len(digits)}", digits
+
+        # Something that announced itself as an ISBN and is not one carries no
+        # information: storing it would link the reader to a page that does not exist.
+        if name is not None:
+            return None
+
+    return name or ("uuid" if _UUID.match(value) else "id"), value
+
+
 class FileExtractor(Protocol):
    @classmethod
    async def extract_metadata(
@@ -363,15 +433,23 @@ class EpubExtractor(FileExtractor):

    @classmethod
    def _extract_identifiers(cls, epub: epub.EpubBook) -> dict[str, str]:
+        """
+        Every `DC:identifier` the file carries, keyed by what kind of thing it is.
+
+        Non-ISBN identifiers are kept: `Identifier` is a free-form name/value pair, so
+        a Calibre id or an ASIN costs nothing to store and is one more thing two copies
+        of a book can be recognised by.
+        """
        identifiers = {}

-        for id in epub.get_metadata("DC", "identifier"):
-            if is_valid_isbn(id[0]):
-                if len(id[0]) == 13:
-                    identifiers.update({"isbn-13": id[0]})
+        for value, attributes in epub.get_metadata("DC", "identifier"):
+            scheme = None
+            if isinstance(attributes, dict):
+                scheme = attributes.get("opf:scheme") or attributes.get("scheme")

-                elif len(id[0]) == 10:
-                    identifiers.update({"isbn-10": id[0]})
+            if (parsed := parse_identifier(value, scheme)) is not None:
+                name, parsed_value = parsed
+                identifiers[name] = parsed_value

        return identifiers

@@ -7,6 +7,7 @@ from chitai.services.matching import (
    normalize_identifier,
    normalize_title,
 )
+from chitai.services.metadata_extractor import parse_identifier
 from chitai.services.utils import isbn10_to_isbn13


@@ -123,3 +124,45 @@ class TestIsbnConversion:
    @pytest.mark.parametrize("isbn", ["0486282113", "9780486282114", "nonsense"])
    def test_anything_that_is_not_an_isbn_10_converts_to_nothing(self, isbn: str) -> None:
        assert isbn10_to_isbn13(isbn) is None
+
+
+class TestParseIdentifier:
+    """What an EPUB writes, and what is worth storing for it."""
+
+    @pytest.mark.parametrize(
+        "written",
+        [
+            "9780486282114",
+            "978-0-486-28211-4",
+            "urn:isbn:9780486282114",
+            "urn:isbn:978-0-486-28211-4",
+            "ISBN:978-0-486-28211-4",
+        ],
+    )
+    def test_isbns_survive_however_they_are_written(self, written: str) -> None:
+        assert parse_identifier(written) == ("isbn-13", "9780486282114")
+
+    def test_the_scheme_attribute_is_read_too(self) -> None:
+        assert parse_identifier("0-486-28211-2", "ISBN") == ("isbn-10", "0486282112")
+
+    def test_non_isbn_identifiers_are_kept(self) -> None:
+        assert parse_identifier("urn:uuid:3f2b1c4e-1111-2222-3333-444455556666") == (
+            "uuid",
+            "3f2b1c4e-1111-2222-3333-444455556666",
+        )
+        assert parse_identifier("calibre:1234") == ("calibre", "1234")
+        assert parse_identifier("B000FC0PDA", "mobi-asin") == ("asin", "B000FC0PDA")
+
+    def test_an_unrecognised_prefix_is_part_of_the_value(self) -> None:
+        """"http://example.com/book" is not an identifier called "http"."""
+        assert parse_identifier("http://www.gutenberg.org/5200") == (
+            "id",
+            "http://www.gutenberg.org/5200",
+        )
+
+    def test_a_declared_isbn_that_is_not_one_is_dropped(self) -> None:
+        assert parse_identifier("urn:isbn:not-an-isbn") is None
+
+    @pytest.mark.parametrize("written", ["", "   ", None])
+    def test_nothing_yields_nothing(self, written: str | None) -> None:
+        assert parse_identifier(written) is None