From c400f8a18dc4f143ee87ce03530214608b772d21 Mon Sep 17 00:00:00 2001 From: Colin Powell Date: Tue, 1 Sep 2026 11:54:14 -0400 Subject: [PATCH] [books] Match books by Google Books ID or author name --- PROJECT.org | 24 +++- tests/scrobbles_tests/test_book_enrichment.py | 129 +++++++++++++++++- .../commands/cleanup_book_metadata.py | 11 +- .../migrations/0040_book_google_books_id.py | 16 +++ vrobbler/apps/books/models.py | 72 ++++++++-- vrobbler/apps/books/sources/google.py | 37 +++-- 6 files changed, 255 insertions(+), 34 deletions(-) create mode 100644 vrobbler/apps/books/migrations/0040_book_google_books_id.py diff --git a/PROJECT.org b/PROJECT.org index bf73c49..41b746e 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -589,16 +589,32 @@ The Edit log form should have from top to bottom: 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: +** DONE [#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. +Turns out we hadn't. Google Books is still a primary metadata source alongside +OpenLibrary and ComicVine. =Book.find_or_create()= only matched by +=original_title=, which is fragile. It now also matches by Google Books ID or +author name (like Track does with =musicbrainz_id=). + *** 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). +- ~vrobbler/apps/books/sources/google.py~: =lookup_book_from_google()= now + accepts an optional =author= (used as an =inauthor:= query term) and returns + the top-level Google Books volume =id= as =google_books_id=. +- ~vrobbler/apps/books/models.py~: added a =google_books_id= CharField to + =Book=. =find_or_create()= now: + - short-circuits on an exact =original_title= match (no lookups); + - after enrichment, reuses an existing book by =google_books_id=, falling + back to a =title= + author match — mirroring how Track keys off + =musicbrainz_id=. + - =fix_metadata()= gained a Google Books fallback so asynchronously enriched + (e.g. KoReader-imported) books also capture =google_books_id=. +- ~vrobbler/apps/books/management/commands/cleanup_book_metadata.py~: passes + the author to the Google lookup and persists =google_books_id= on backfill. +- Migration ~0040_book_google_books_id~ adds the field. ** DONE [#B] Books created via koreader do not get enriched :books:metadata:bug: :PROPERTIES: diff --git a/tests/scrobbles_tests/test_book_enrichment.py b/tests/scrobbles_tests/test_book_enrichment.py index 8824b5d..c0b39bc 100644 --- a/tests/scrobbles_tests/test_book_enrichment.py +++ b/tests/scrobbles_tests/test_book_enrichment.py @@ -28,7 +28,11 @@ def test_enrich_book_metadata_tags_success(mock_lookup): @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): +@patch("books.models.lookup_comic_from_locg", return_value={}) +@patch("books.models.lookup_book_from_google", return_value={}) +def test_enrich_book_metadata_tags_failure_when_no_match( + mock_google, mock_locg, mock_ol +): book = Book.objects.create(title="Unknown Book", pages=100) enrich_book_metadata(book.id) @@ -40,7 +44,11 @@ def test_enrich_book_metadata_tags_failure_when_no_match(mock_lookup): @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): +@patch("books.models.lookup_comic_from_locg", side_effect=Exception("boom")) +@patch("books.models.lookup_book_from_google", side_effect=Exception("boom")) +def test_enrich_book_metadata_tags_failure_on_exception( + mock_google, mock_locg, mock_ol +): book = Book.objects.create(title="Test Book", pages=100) enrich_book_metadata(book.id) @@ -113,3 +121,120 @@ def test_fix_metadata_does_not_crash_on_locg_data_with_isbn(): assert enriched is True book.refresh_from_db() assert book.summary == "A comic summary" + + +@pytest.mark.django_db +def test_find_or_create_returns_existing_by_title(): + book = Book.objects.create(original_title="Dune", title="Dune") + with patch("books.models.lookup_book_from_google") as mock_google: + found = Book.find_or_create("Dune") + assert found.id == book.id + mock_google.assert_not_called() + + +@pytest.mark.django_db +@patch("books.models.lookup_comic_from_comicvine", return_value={}) +@patch("books.models.lookup_book_from_ol", return_value={}) +@patch("books.models.lookup_book_from_google") +def test_find_or_create_reuses_existing_by_google_books_id( + mock_google, mock_ol, mock_comicvine +): + existing = Book.objects.create( + original_title="Dune - 50th Anniversary", + title="Dune", + google_books_id="gbooks_123", + ) + mock_google.return_value = { + "title": "Dune", + "google_books_id": "gbooks_123", + "authors": ["Frank Herbert"], + "pages": 412, + } + + found = Book.find_or_create("Dune") + + assert found.id == existing.id + assert Book.objects.filter(original_title="Dune").count() == 0 + assert Book.objects.count() == 1 + assert found.authors.filter(name="Frank Herbert").exists() + + +@pytest.mark.django_db +@patch("books.models.lookup_comic_from_comicvine", return_value={}) +@patch("books.models.lookup_book_from_ol", return_value={}) +@patch("books.models.lookup_book_from_google") +def test_find_or_create_matches_by_title_and_author( + mock_google, mock_ol, mock_comicvine +): + author = Author.objects.create(name="Frank Herbert") + existing = Book.objects.create(original_title="Dune - Part 1", title="Dune") + existing.authors.add(author) + mock_google.return_value = { + "title": "Dune", + "authors": ["Frank Herbert"], + "pages": 412, + } + + found = Book.find_or_create("Dune", author="Frank Herbert") + + assert found.id == existing.id + assert Book.objects.count() == 1 + + +@pytest.mark.django_db +@patch("books.models.lookup_comic_from_comicvine", return_value={}) +@patch("books.models.lookup_book_from_ol", return_value={}) +@patch("books.models.lookup_book_from_google") +def test_find_or_create_stores_google_books_id(mock_google, mock_ol, mock_comicvine): + mock_google.return_value = { + "title": "Dune", + "google_books_id": "gbooks_456", + "authors": ["Frank Herbert"], + "pages": 412, + } + + book = Book.find_or_create("Dune") + + assert book.google_books_id == "gbooks_456" + assert book.authors.filter(name="Frank Herbert").exists() + + +@patch("books.sources.google.requests.get") +def test_lookup_book_from_google_captures_volume_id(mock_get): + from books.sources.google import lookup_book_from_google + + mock_response = mock_get.return_value + mock_response.status_code = 200 + mock_response.content = b""" + {"items": [ + { + "id": "gbooks_789", + "volumeInfo": { + "title": "Dune", + "authors": ["Frank Herbert"], + "publishedDate": "1965", + "pageCount": 412 + } + } + ]} + """ + + result = lookup_book_from_google("Dune") + + assert result["google_books_id"] == "gbooks_789" + assert result["authors"] == ["Frank Herbert"] + assert result["first_publish_year"] == 1965 + + +@patch("books.sources.google.requests.get") +def test_lookup_book_from_google_includes_author_in_query(mock_get): + from books.sources.google import lookup_book_from_google + + mock_response = mock_get.return_value + mock_response.status_code = 200 + mock_response.content = b'{"items": []}' + + lookup_book_from_google("Dune", author="Frank Herbert") + + args, kwargs = mock_get.call_args + assert "inauthor:Frank Herbert" in kwargs["params"]["q"] diff --git a/vrobbler/apps/books/management/commands/cleanup_book_metadata.py b/vrobbler/apps/books/management/commands/cleanup_book_metadata.py index 851db14..91d4222 100644 --- a/vrobbler/apps/books/management/commands/cleanup_book_metadata.py +++ b/vrobbler/apps/books/management/commands/cleanup_book_metadata.py @@ -18,6 +18,7 @@ MISSING_ALL = [ "publish_year", ] + def _cover_missing_or_broken(book) -> bool: if not bool(book.cover): return True @@ -174,7 +175,9 @@ class Command(BaseCommand): lookup_issue_by_comicvine_id, ) from books.sources.google import lookup_book_from_google - from books.sources.openlibrary import lookup_book_from_openlibrary as lookup_book_from_ol + from books.sources.openlibrary import ( + lookup_book_from_openlibrary as lookup_book_from_ol, + ) title = book.original_title or book.title author_name = book.author.name if book.author else None @@ -190,7 +193,7 @@ class Command(BaseCommand): ol_data = lookup_book_from_ol(title, author=author_name) time.sleep(sleep_secs) - google_data = lookup_book_from_google(title) + google_data = lookup_book_from_google(title, author=author_name) if ol_data: for k, v in ol_data.items(): @@ -261,6 +264,10 @@ class Command(BaseCommand): book.openlibrary_id = data["openlibrary_id"] update_fields.append("openlibrary_id") + if data.get("google_books_id") and not book.google_books_id: + book.google_books_id = data["google_books_id"] + update_fields.append("google_books_id") + if data.get("comicvine_id") and not book.comicvine_id: book.comicvine_id = data["comicvine_id"] update_fields.append("comicvine_id") diff --git a/vrobbler/apps/books/migrations/0040_book_google_books_id.py b/vrobbler/apps/books/migrations/0040_book_google_books_id.py new file mode 100644 index 0000000..7cb883b --- /dev/null +++ b/vrobbler/apps/books/migrations/0040_book_google_books_id.py @@ -0,0 +1,16 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("books", "0039_journal_alter_paper_journal"), + ] + + operations = [ + migrations.AddField( + model_name="book", + name="google_books_id", + field=models.CharField(blank=True, max_length=255, null=True), + ), + ] diff --git a/vrobbler/apps/books/models.py b/vrobbler/apps/books/models.py index a6dbd66..08dfb15 100644 --- a/vrobbler/apps/books/models.py +++ b/vrobbler/apps/books/models.py @@ -124,9 +124,11 @@ class Author(TimeStampedModel): def __str__(self): return f"{self.name}" - def enrich_from_semantic(self, overwrite=False): ... + def enrich_from_semantic(self, overwrite=False): + ... - def enrich_from_google_books(self, overwrite=False): ... + def enrich_from_google_books(self, overwrite=False): + ... def enrich_from_openlibrary(self, overwrite=False): data_dict = lookup_author_from_openlibrary(self.openlibrary_id) @@ -173,6 +175,8 @@ class Book(LongPlayScrobblableMixin): volume_number = models.IntegerField(**BNULL) # OpenLibrary openlibrary_id = models.CharField(max_length=255, **BNULL) + # Google Books + google_books_id = models.CharField(max_length=255, **BNULL) cover = models.ImageField(upload_to="books/covers/", **BNULL) cover_small = ImageSpecField( source="cover", @@ -238,7 +242,9 @@ class Book(LongPlayScrobblableMixin): @property def resume_start_url(self): - return reverse("scrobbles:start", kwargs={"media_uuid": self.uuid}) + "?resume=1" + return ( + reverse("scrobbles:start", kwargs={"media_uuid": self.uuid}) + "?resume=1" + ) @classmethod def get_from_comicvine( @@ -284,17 +290,18 @@ class Book(LongPlayScrobblableMixin): By default this method will also save the data back to the model. If you'd like to batch create, use commit=False and you'll get an unsaved but enriched instance back which you can then save at your convenience.""" - # TODO use either a Google Books id identifier or author name like for tracks - book, created = cls.objects.get_or_create(original_title=title) - if not created: + # Fast path: an exact title match needs no lookups. + book = cls.objects.filter(original_title=title).first() + if book: logger.info("Found exact match for book by title", extra={"title": title}) + return book if not enrich: logger.info( - "Found book by title, but not enriching", + "No existing book by title, not enriching", extra={"title": title}, ) - return book + return cls.objects.create(original_title=title) book_dict = None source_tag = None @@ -312,7 +319,7 @@ class Book(LongPlayScrobblableMixin): if not book_dict: book_dict = {} ol_data = lookup_book_from_ol(title, author=author) - google_data = lookup_book_from_google(title) + google_data = lookup_book_from_google(title, author=author) if ol_data: book_dict.update(ol_data) @@ -358,12 +365,43 @@ class Book(LongPlayScrobblableMixin): if authors: for author_str in authors: if author_str: - author, a_created = Author.objects.get_or_create(name=author_str) - author_list.append(author) + author_obj, a_created = Author.objects.get_or_create( + name=author_str + ) + author_list.append(author_obj) if a_created: # TODO enrich author ... + # Match an existing book harder than a plain title would, mirroring how + # Track reuses records keyed by musicbrainz_id. + google_books_id = book_dict.get("google_books_id") + if google_books_id: + book = cls.objects.filter(google_books_id=google_books_id).first() + if book: + logger.info( + "Found existing book by Google Books ID", + extra={"title": title, "google_books_id": google_books_id}, + ) + + if not book and author_list: + book = ( + cls.objects.filter(title=title, authors__in=author_list) + .distinct() + .first() + ) + if book: + logger.info( + "Found existing book by title and author", + extra={"title": title}, + ) + + if not book: + book = cls(original_title=title) + if commit: + book.save() + book.refresh_from_db() + for k, v in book_dict.items(): setattr(book, k, v) @@ -407,14 +445,16 @@ class Book(LongPlayScrobblableMixin): if not data and COMICVINE_API_KEY: if self.comicvine_id: - logger.warn( - f"Checking ComicVine by ID for {self.title}" - ) + logger.warn(f"Checking ComicVine by ID for {self.title}") data = lookup_issue_by_comicvine_id(str(self.comicvine_id)) if not data: logger.warn(f"Checking ComicVine for {self.title}") data = lookup_comic_from_comicvine(str(self.title)) + if not data: + logger.warn(f"Checking Google Books for {self.title}") + data = lookup_book_from_google(str(self.title), author=author_name) + if not data: logger.warn(f"Book not found in any sources: {self.title}") return False @@ -527,7 +567,9 @@ class Book(LongPlayScrobblableMixin): tz = None if scrobble.timezone: tz = ZoneInfo(scrobble.timezone) - data["start_ts"] = datetime.fromtimestamp(data["start_ts"], tz=tz) + data["start_ts"] = datetime.fromtimestamp( + data["start_ts"], tz=tz + ) data["end_ts"] = datetime.fromtimestamp(data["end_ts"], tz=tz) pages[page] = data sorted_pages = OrderedDict( diff --git a/vrobbler/apps/books/sources/google.py b/vrobbler/apps/books/sources/google.py index c20742f..81d485c 100644 --- a/vrobbler/apps/books/sources/google.py +++ b/vrobbler/apps/books/sources/google.py @@ -6,26 +6,41 @@ import requests from django.conf import settings API_KEY = settings.GOOGLE_API_KEY -GOOGLE_BOOKS_URL = 'https://www.googleapis.com/books/v1/volumes?q="{title}"&key={key}' +GOOGLE_BOOKS_URL = "https://www.googleapis.com/books/v1/volumes" logger = logging.getLogger(__name__) -def lookup_book_from_google(title: str) -> dict: +def lookup_book_from_google(title: str, author: str | None = None) -> dict: book_dict = {"title": title} - url = GOOGLE_BOOKS_URL.format(title=title, key=API_KEY) + query = f'"{title}"' + if author: + query += f" inauthor:{author}" + headers = {"User-Agent": "Vrobbler 0.11.12"} - response = requests.get(url, headers=headers) - - if response.status_code != 200: - logger.warning("Bad response from Google", extra={"response": response}) - return book_dict - - google_result = json.loads(response.content).get("items", [{}])[0].get("volumeInfo") - if not google_result: + try: + response = requests.get( + GOOGLE_BOOKS_URL, + params={"q": query, "key": API_KEY}, + headers=headers, + timeout=10, + ) + response.raise_for_status() + except Exception as e: + logger.warning(f"Could not search for {title}: {e}") return {} + items = json.loads(response.content).get("items", []) + if not items: + return {} + + item = items[0] + book_dict["google_books_id"] = item.get("id") + google_result = item.get("volumeInfo") + if not google_result: + return book_dict + isbn_13 = "" isbn_10 = "" for ident in google_result.get("industryIdentifiers", []):