[books] Enrich KoReader-imported books asynchronously

PROJECT.org

@@ -18,7 +18,7 @@ tasks, Todoist tasks, web pages I've read and trails I've hiked has turned out
 to be sometimes cathartic and sometimes functional as I try to remember when I
 did a thing.
 
-* Backlog [0/30] :vrobbler:project:personal:
+* Backlog [1/35] :vrobbler:project:personal:
 ** TODO [#C] Configure IMAP folder/start in user profile :imap:settings:
 *** Description
 
@@ -490,17 +490,6 @@ File: ~vrobbler/apps/scrobbles/scrobblers.py~ (line 1245)
 - =fetch_file_from_webdav()= returns an empty string on failure, which could
     silently propagate. Should raise an exception for proper error handling.
 
-** TODO [#B] Make book matching use Google Books ID or author name :books:matching:
-*** Description
-
-This may be a no-op situation as we've moved away from Google Books. But maybe we haven't.
-
-*** Implementation
-
-- File: ~vrobbler/apps/books/models.py~ (line 270)
-- =Book.find_or_create()= only matches by =original_title=, which is fragile.
-    Should also match by Google Books ID or author name (like Track does).
-
 ** TODO [#B] Add AllTrails as a source for Trail data :trails:feature:
 :PROPERTIES:
 :ID:       39313362-cdfe-46e7-bbd4-9139a65c0b3c
@@ -590,4 +579,74 @@ The Edit log form should have from top to bottom:
   - People (which should be similar to the Bird widget on BirdLocation and allow setting per user score, win true/false, rank, new true/false, seat_ordrer)
   - Expansion ids (which should a multi-select widget of expansions for this game)
   - Location (which should be a drop down of BoardGameLocations for this user)
+** TODO [#B] Remove vestigial "Grid view" from all templates :templates:ui:
+** TODO [#B] Fix links on book page to sources :ui:templates:bug:
+:PROPERTIES:
+:ID:       d54e6350-6200-4c04-95c9-4ff82448307a
+:END:
 
+*** Description
+
+Currently the images on the book source page go nowhere. We should add links to Amazon and Goodreads, and maybe Bookshop.org if we can find a way to generate that link easily.
+
+** TODO [#B] Make book matching use Google Books ID or author name :bug:books:matching:
+*** Description
+
+This may be a no-op situation as we've moved away from Google Books. But maybe we haven't.
+
+*** Implementation
+
+- File: ~vrobbler/apps/books/models.py~ (line 270)
+- =Book.find_or_create()= only matches by =original_title=, which is fragile.
+    Should also match by Google Books ID or author name (like Track does).
+
+** DONE [#B] Books created via koreader do not get enriched :books:metadata:bug:
+:PROPERTIES:
+:ID:       ddfed612-1f4e-b0bb-5310-19adf9cf9336
+:END:
+
+*** Description
+
+When a new books arrives inside of a KoReader import, the metadata is not enriched.
+
+*** Implementation
+
+- Added =scrobbles.tasks.enrich_book_metadata()=, a background Celery task that
+  calls =Book.fix_metadata()= and tags the book =book-enriched= on success or
+  =enrichment-failed= on failure.
+- =create_book_from_row()= in ~vrobbler/apps/books/koreader.py~ now dispatches
+  that task asynchronously after creating a new book, instead of the commented
+  out synchronous =fix_metadata()= call.
+- Fixed =Book.fix_metadata()= to not reference the removed =locg_slug= field,
+  and to handle =authors=/=genres= keys returned by the current OpenLibrary
+  source (previously it would crash).
+- Routed =enrich_book_metadata= to the =background= worker queue.
+
+** TODO [#B] There should a global chart aggregator that builds charts across all users :charts:feature:
+:PROPERTIES:
+:ID:       ccc257d5-7d35-42b0-9af2-2941b67880fd
+:END:
+
+*** Description
+
+Currently we build charts for individual users ... noting content that has been
+tops for a given year, month, week or day. We should also build such global
+charts across all users, but limit it to just the top three items (gold, silver,
+bronze). There should also be a toggle in profile settings that allows a user to
+include their scrobbles in global aggregation. If that check box is not selected
+(and it should off by default) that user is not included in the global charts.
+
+
+** DONE [#B] Make it possible for anonymous users to view media objects :media:templates:
+:PROPERTIES:
+:ID:       f12fd0b4-b728-4fbc-adf7-d51695d8c155
+:END:
+
+*** Description
+
+Currently if you click on a Book link when logged out (like via a shared URL: https://life.lab.unbl.ink/shared/KkViBro/ ) you get a 500 error due to the anonymous user not having some properties of an authenticated user. Anonymous users should be able view media on the site, just without an scrobble data being present.
+
+*** Error
+#+begin_src json
+{"levelname": "ERROR", "name": "django.request", "funcName": "log_response", "lineno": 241, "asctime": "2026-09-01 08:49:52,844", "message": "Internal Server Error: /books/0879dd15-fe34-4ed4-b6b0-f68c924aed23/", "exc_info": "Traceback (most recent call last):\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/fields/__init__.py\", line 2053, in get_prep_value\n    return int(value)\n           ^^^^^^^^^^\nTypeError: int() argument must be a string, a bytes-like object or a real number, not 'SimpleLazyObject'\n\nThe above exception was the direct cause of the following exception:\n\nTraceback (most recent call last):\n  File \"/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py\", line 55, in inner\n    response = get_response(request)\n               ^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py\", line 197, in _get_response\n    response = wrapped_callback(request, *callback_args, **callback_kwargs)\n               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/views/generic/base.py\", line 104, in view\n    return self.dispatch(request, *args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/views/generic/base.py\", line 143, in dispatch\n    return handler(request, *args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/views/generic/detail.py\", line 109, in get\n    context = self.get_context_data(object=self.object)\n              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/vrobbler/apps/scrobbles/views.py\", line 219, in get_context_data\n    qs = media.scrobble_set.filter(user=self.request.user)\n         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/manager.py\", line 87, in manager_method\n    return getattr(self.get_queryset(), name)(*args, **kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/query.py\", line 1436, in filter\n    return self._filter_or_exclude(False, args, kwargs)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/query.py\", line 1454, in _filter_or_exclude\n    clone._filter_or_exclude_inplace(negate, args, kwargs)\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/query.py\", line 1461, in _filter_or_exclude_inplace\n    self._query.add_q(Q(*args, **kwargs))\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/sql/query.py\", line 1546, in add_q\n    clause, _ = self._add_q(q_object, self.used_aliases)\n                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/sql/query.py\", line 1577, in _add_q\n    child_clause, needed_inner = self.build_filter(\n                                 ^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/sql/query.py\", line 1492, in build_filter\n    condition = self.build_lookup(lookups, col, value)\n                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/sql/query.py\", line 1319, in build_lookup\n    lookup = lookup_class(lhs, rhs)\n             ^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/lookups.py\", line 27, in __init__\n    self.rhs = self.get_prep_lookup()\n               ^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/fields/related_lookups.py\", line 166, in get_prep_lookup\n    self.rhs = target_field.get_prep_value(self.rhs)\n               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/usr/local/lib/python3.11/site-packages/django/db/models/fields/__init__.py\", line 2055, in get_prep_value\n    raise e.__class__(\nTypeError: Field 'id' expected a number but got <SimpleLazyObject: <django.contrib.auth.models.AnonymousUser object at 0x8ffebe31690>>.", "status_code": 500, "request": "<WSGIRequest: GET '/books/0879dd15-fe34-4ed4-b6b0-f68c924aed23/'>"}
+#+end_src

tests/scrobbles_tests/test_book_enrichment.py

@@ -0,0 +1,115 @@
+from unittest.mock import patch
+
+import pytest
+from books.models import Author, Book
+from scrobbles.tasks import enrich_book_metadata
+
+
+@pytest.mark.django_db
+@patch("books.models.lookup_book_from_ol")
+def test_enrich_book_metadata_tags_success(mock_lookup):
+    mock_lookup.return_value = {
+        "title": "Test Book",
+        "openlibrary_id": "OL12345",
+        "summary": "A test summary",
+        "authors": ["Test Author"],
+    }
+    book = Book.objects.create(title="Test Book", pages=100)
+
+    enrich_book_metadata(book.id)
+
+    book.refresh_from_db()
+    assert book.openlibrary_id == "OL12345"
+    assert book.summary == "A test summary"
+    assert book.authors.filter(name="Test Author").exists()
+    assert "book-enriched" in book.tags.names()
+    assert "enrichment-failed" not in book.tags.names()
+
+
+@pytest.mark.django_db
+@patch("books.models.lookup_book_from_ol", return_value={})
+def test_enrich_book_metadata_tags_failure_when_no_match(mock_lookup):
+    book = Book.objects.create(title="Unknown Book", pages=100)
+
+    enrich_book_metadata(book.id)
+
+    book.refresh_from_db()
+    assert "book-enriched" not in book.tags.names()
+    assert "enrichment-failed" in book.tags.names()
+
+
+@pytest.mark.django_db
+@patch("books.models.lookup_book_from_ol", side_effect=Exception("boom"))
+def test_enrich_book_metadata_tags_failure_on_exception(mock_lookup):
+    book = Book.objects.create(title="Test Book", pages=100)
+
+    enrich_book_metadata(book.id)
+
+    book.refresh_from_db()
+    assert "book-enriched" not in book.tags.names()
+    assert "enrichment-failed" in book.tags.names()
+
+
+@pytest.mark.django_db
+def test_enrich_book_metadata_skips_missing_book():
+    enrich_book_metadata(99999)
+
+
+@pytest.mark.django_db
+def test_enrich_book_metadata_preserves_koreader_title_on_mismatch():
+    from unittest.mock import Mock
+
+    with patch("books.models.lookup_book_from_ol") as mock_lookup:
+        mock_lookup.return_value = {
+            "title": "Completely Different Title",
+            "openlibrary_id": "OL999",
+        }
+        book = Book.objects.create(title="Test Book", pages=100)
+
+        enrich_book_metadata(book.id)
+
+        book.refresh_from_db()
+        assert book.title == "Test Book"
+        assert book.openlibrary_id == "OL999"
+        assert "book-enriched" in book.tags.names()
+
+
+@pytest.mark.django_db
+def test_create_book_from_row_dispatches_enrichment():
+    from books.koreader import create_book_from_row
+
+    row = [
+        1,
+        "Test Book - Author Name",
+        "Test Author",
+        0,
+        0,
+        0,
+        300,
+        "",
+        "",
+        "abc123def456",
+        3600,
+        120,
+    ]
+    with patch("scrobbles.tasks.enrich_book_metadata.delay") as mock_delay:
+        book = create_book_from_row(row)
+        mock_delay.assert_called_once_with(book.id)
+
+
+@pytest.mark.django_db
+def test_fix_metadata_does_not_crash_on_locg_data_with_isbn():
+    from unittest.mock import Mock
+
+    with patch("books.models.lookup_book_from_ol", return_value={}):
+        with patch("books.models.lookup_comic_from_locg") as mock_locg:
+            mock_locg.return_value = {
+                "title": "Test Book",
+                "summary": "A comic summary",
+                "isbn": "1234567890",
+            }
+            book = Book.objects.create(title="Test Book", pages=100)
+            enriched = book.fix_metadata()
+            assert enriched is True
+            book.refresh_from_db()
+            assert book.summary == "A comic summary"

vrobbler/apps/books/koreader.py

@@ -116,14 +116,23 @@ def create_book_from_row(row: list):
         },
         base_run_time_seconds=run_time,
     )
-    # TODO Move these to async processes after importing
-    # book.fix_metadata()
 
     # Add authors
     author_list = lookup_or_create_authors_from_author_str(author_str)
     if author_list:
         book.authors.add(*author_list)
 
+    # Enrich new books asynchronously so imports stay fast
+    try:
+        from scrobbles.tasks import enrich_book_metadata
+
+        enrich_book_metadata.delay(book.id)
+    except Exception as e:
+        logger.warning(
+            "Failed to queue async book enrichment",
+            extra={"book_id": book.id, "error": str(e)},
+        )
+
     # self._lookup_authors
     return book
 

vrobbler/apps/books/models.py

@@ -9,7 +9,6 @@ from zoneinfo import ZoneInfo
 import requests
 from books.constants import MediaSourceTag, READCOMICSONLINE_URL
 from books.locg import (
-    lookup_comic_by_locg_slug,
     lookup_comic_from_locg,
     lookup_comic_writer_by_locg_slug,
 )
@@ -389,8 +388,8 @@ class Book(LongPlayScrobblableMixin):
                 fname = f"{self.title}_{self.uuid}.jpg"
                 self.cover.save(fname, ContentFile(r.content), save=True)
 
-    def fix_metadata(self, data: dict = {}, force_update=False):
+    def fix_metadata(self, data: dict = {}, force_update=False) -> bool:
-        if (not self.openlibrary_id or not self.locg_slug) or force_update:
+        if not self.openlibrary_id or force_update:
             author_name = ""
             if self.author:
                 author_name = self.author.name
@@ -403,14 +402,8 @@ class Book(LongPlayScrobblableMixin):
                     data = lookup_book_from_ol(str(self.title), author_name)
 
             if not data:
-                if self.locg_slug:
+                logger.warn(f"Checking LOCG for {self.title}")
-                    logger.warn(
+                data = lookup_comic_from_locg(str(self.title))
-                        f"Checking LOCG for {self.title} with slug {self.locg_slug}"
-                    )
-                    data = lookup_comic_by_locg_slug(str(self.locg_slug))
-                else:
-                    logger.warn(f"Checking LOCG for {self.title}")
-                    data = lookup_comic_from_locg(str(self.title))
 
             if not data and COMICVINE_API_KEY:
                 if self.comicvine_id:
@@ -424,14 +417,12 @@ class Book(LongPlayScrobblableMixin):
 
             if not data:
                 logger.warn(f"Book not found in any sources: {self.title}")
-                return
+                return False
 
             # We can discard the author name from OL for now, we'll lookup details below
             data.pop("ol_author_name", "")
             if data.get("ol_author_id"):
                 self.fix_authors_metadata(data.pop("ol_author_id", ""))
-            if data.get("locg_writer_slug"):
-                self.get_author_from_locg(data.pop("locg_writer_slug", ""))
 
             ol_title = data.get("title", "")
             data.pop("ol_author_id", "")
@@ -446,7 +437,7 @@ class Book(LongPlayScrobblableMixin):
                 )
                 self.openlibrary_id = data.get("openlibrary_id")
                 self.save(update_fields=["openlibrary_id"])
-                return
+                return True
 
             # If we don't know pages, don't overwrite existing with None
             if "pages" in data.keys() and data.get("pages") == None:
@@ -461,16 +452,32 @@ class Book(LongPlayScrobblableMixin):
             # Pop these so they don't get passed to update()
             cover_url = data.pop("cover_url", "")
             subject_key_list = data.pop("subject_key_list", [])
+            genres = data.pop("genres", data.pop("generes", []))
             tags = data.pop("tags", [])
+            authors = data.pop("authors", [])
+            if data.get("locg_writer_slug"):
+                self.get_author_from_locg(data.pop("locg_writer_slug", ""))
+
+            author_list = []
+            for author_str in authors:
+                if author_str:
+                    author, _ = Author.objects.get_or_create(name=author_str)
+                    author_list.append(author)
 
             # Fun trick for updating all fields at once
-            Book.objects.filter(pk=self.id).update(**data)
+            field_names = {f.name for f in Book._meta.concrete_fields}
+            update_data = {k: v for k, v in data.items() if k in field_names}
+            Book.objects.filter(pk=self.id).update(**update_data)
             self.refresh_from_db()
 
             if subject_key_list:
                 self.genre.add(*subject_key_list)
+            if genres:
+                self.genre.add(*genres)
             if tags:
                 self.tags.add(*tags)
+            if author_list:
+                self.authors.add(*author_list)
 
             if cover_url:
                 r = requests.get(cover_url)
@@ -479,6 +486,9 @@ class Book(LongPlayScrobblableMixin):
                     self.cover.save(fname, ContentFile(r.content), save=True)
 
             self.save()
+            return True
+
+        return True
 
     def fix_authors_metadata(self, openlibrary_author_id):
         author = Author.objects.filter(openlibrary_id=openlibrary_author_id).first()
@@ -498,9 +508,7 @@ class Book(LongPlayScrobblableMixin):
     def get_author_from_locg(self, locg_slug):
         writer = lookup_comic_writer_by_locg_slug(locg_slug)
 
-        author, created = Author.objects.get_or_create(
+        author, created = Author.objects.get_or_create(name=writer["name"])
-            name=writer["name"], locg_slug=writer["locg_slug"]
-        )
         if (created or not author.headshot) and writer["photo_url"]:
             r = requests.get(writer["photo_url"])
             if r.status_code == 200:

vrobbler/apps/scrobbles/constants.py

@@ -18,6 +18,7 @@ JELLYFIN_AUDIO_ITEM_TYPES = ["Audio"]
 # scrobble to WEBHOOK_ENRICHED_TAG, failure to ENRICHMENT_FAILED_TAG.
 WEBHOOK_METADATA_ONLY_TAG = "webhook-metadata-only"
 WEBHOOK_ENRICHED_TAG = "webhook-enriched"
+BOOK_ENRICHED_TAG = "book-enriched"
 ENRICHMENT_FAILED_TAG = "enrichment-failed"
 
 LONG_PLAY_MEDIA = {

vrobbler/apps/scrobbles/tasks.py

@@ -520,6 +520,52 @@ def enrich_media_from_webhook(scrobble_id):
         scrobble.tags.add(ENRICHMENT_FAILED_TAG)
 
 
+@shared_task
+def enrich_book_metadata(book_id):
+    """Enrich a Book's metadata asynchronously after import.
+
+    Importers like KoReader create bare book records; external metadata
+    (OpenLibrary, LOCG, ComicVine) is fetched in the background so the import
+    itself stays fast. The book is tagged ``book-enriched`` on success or
+    ``enrichment-failed`` on failure.
+    """
+    from books.models import Book
+    from scrobbles.constants import BOOK_ENRICHED_TAG, ENRICHMENT_FAILED_TAG
+
+    book = Book.objects.filter(id=book_id).first()
+    if not book:
+        logger.warning(
+            "enrich_book_metadata: book not found",
+            extra={"book_id": book_id},
+        )
+        return
+
+    logger.info(
+        "enrich_book_metadata: enriching book",
+        extra={"book_id": book_id, "title": book.title},
+    )
+    try:
+        enriched = book.fix_metadata()
+        if enriched:
+            book.tags.add(BOOK_ENRICHED_TAG)
+            logger.info(
+                "enrich_book_metadata: enrichment complete",
+                extra={"book_id": book_id, "title": book.title},
+            )
+        else:
+            book.tags.add(ENRICHMENT_FAILED_TAG)
+            logger.warning(
+                "enrich_book_metadata: no enrichment match",
+                extra={"book_id": book_id, "title": book.title},
+            )
+    except Exception:
+        logger.exception(
+            "enrich_book_metadata: enrichment failed",
+            extra={"book_id": book_id, "title": book.title},
+        )
+        book.tags.add(ENRICHMENT_FAILED_TAG)
+
+
 # ── Crontab replacements ──────────────────────────────────────────────────────
 
 

vrobbler/settings.py

@@ -213,6 +213,7 @@ CELERY_TASK_ROUTES = {
     "scrobbles.tasks.backup_database": {"queue": "background"},
     "scrobbles.tasks.backfill_scrobble_sentiment": {"queue": "background"},
     "scrobbles.tasks.enrich_media_from_webhook": {"queue": "background"},
+    "scrobbles.tasks.enrich_book_metadata": {"queue": "background"},
     "scrobbles.tasks.check_twitch_channels_for_vods": {"queue": "background"},
     "scrobbles.importers.lastfm.dispatch_historical_imports": {"queue": "background"},
     "trends.tasks.compute_all_trends": {"queue": "background"},