fix(metadata): fix metadata fetching relying for filename "first" instead of better fields like ISBN in Goodreads/Bookdrop#85
Open
balazs-szucs wants to merge 8 commits intogrimmory-tools:developfrom
Conversation
Dependabot couldn't find the original pull request head commit, ea510f4. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…immory-tools#2) Dependabot couldn't find the original pull request head commit, faed6bf. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…mory-tools#3) Dependabot couldn't find the original pull request head commit, f110823. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…tead of better fields like ISBN in Goodreads/Bookdrop
- Image builds: - pin frontend and backend Docker build stages to the build platform so multi-arch packaging can reuse architecture-independent work - add an Angular build cache mount and move dynamic version metadata to the end of the runtime stage so static layers stay reusable across tags - reduce image workflow checkout depth and keep preview builds on `linux/amd64` only to avoid unnecessary QEMU and history overhead - Cache policy: - make CI packaging smoke builds consume the shared image cache without writing new BuildKit cache state - make normal preview builds consume shared GHA and registry caches without mutating the canonical cache - keep nightly and stable release builds as the workflows that refresh the shared image cache in both GHA and registry backends - Preview override: - add a `refresh_shared_cache` input to the preview workflow for an explicit no-cache rebuild that repopulates the shared cache when maintainers need to bust and refresh it - keep the default preview behavior optimized for fast disposable builds rather than cache churn - Validation: - keep workflow YAML parsing clean after the cache-policy changes - keep `git diff --check` clean for the touched Docker and workflow files
Contributor
📝 WalkthroughWalkthroughVersion 2.2.2 release documentation added to CHANGELOG. BookdropMetadataService refactored to conditionally skip online metadata refresh for non-searchable entries. GoodReadsParser enhanced with ISBN-based lookup path and centralized ID extraction helper. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
CHANGELOG.md (1)
1-12: Move# Changelogback to the top of the file.The top-level heading should be the first line for consistent changelog structure/readability (and better compatibility with tooling that expects it there).
Proposed reorder
-## [2.2.2](https://github.com/grimmory-tools/grimmory/compare/v2.2.1...v2.2.2) (2026-03-19) - -### Bug Fixes - -* **ci:** forward checks permission to reusable test jobs ([e3b3e7c](https://github.com/grimmory-tools/grimmory/commit/e3b3e7c4d7aae23f517b6029af56df360d40edf1)) - -### Chores - -* **ci:** rebrand and streamline the release pipeline ([`#66`](https://github.com/grimmory-tools/grimmory/issues/66)) ([1bc45da](https://github.com/grimmory-tools/grimmory/commit/1bc45da013415dc66621dc72eb21be9640028f4d)) -* Update README to use new links, and clean up formatting ([6b0c4aa](https://github.com/grimmory-tools/grimmory/commit/6b0c4aa66c2247de0335340dbae8ec0afc1fbd59)) - # Changelog All notable Grimmory releases are tracked here. This file is maintained by semantic-release. + +## [2.2.2](https://github.com/grimmory-tools/grimmory/compare/v2.2.1...v2.2.2) (2026-03-19) + +### Bug Fixes + +* **ci:** forward checks permission to reusable test jobs ([e3b3e7c](https://github.com/grimmory-tools/grimmory/commit/e3b3e7c4d7aae23f517b6029af56df360d40edf1)) + +### Chores + +* **ci:** rebrand and streamline the release pipeline ([`#66`](https://github.com/grimmory-tools/grimmory/issues/66)) ([1bc45da](https://github.com/grimmory-tools/grimmory/commit/1bc45da013415dc66621dc72eb21be9640028f4d)) +* Update README to use new links, and clean up formatting ([6b0c4aa](https://github.com/grimmory-tools/grimmory/commit/6b0c4aa66c2247de0335340dbae8ec0afc1fbd59))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 1 - 12, Move the top-level heading "# Changelog" to the very first line of the file so the file begins with that heading, followed by a single blank line and then the existing release entries (starting with "## [2.2.2]..."). Ensure the "# Changelog" line is removed from its current location and placed above the "## [2.2.2]" section so tooling and readability are consistent.booklore-api/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java (1)
137-148: Good refactor to use the centralized helper.The refactoring to use
extractGoodreadsIdFromOgUrlis consistent with the new approach infetchTopMetadata.However, note that unlike
fetchTopMetadata(lines 74-86), there's notry-catcharound thefetchDoc()call on line 140. IffetchDoc()throws aRuntimeException(which it does onIOException), the entire method fails instead of falling back to the preview search. Consider wrapping this block in a try-catch for consistency.♻️ Optional: Add error handling for consistency with fetchTopMetadata
String isbn = ParserUtils.cleanIsbn(fetchMetadataRequest.getIsbn()); if (isbn != null && !isbn.isBlank()) { log.info("Goodreads Query URL (ISBN): {}{}", BASE_ISBN_URL, isbn); - Document doc = fetchDoc(BASE_ISBN_URL + isbn); - String goodreadsId = extractGoodreadsIdFromOgUrl(doc); - if (goodreadsId != null) { - BookMetadata metadata = parseBookDetails(doc, goodreadsId); - if (metadata != null) { - return List.of(metadata); + try { + Document doc = fetchDoc(BASE_ISBN_URL + isbn); + String goodreadsId = extractGoodreadsIdFromOgUrl(doc); + if (goodreadsId != null) { + BookMetadata metadata = parseBookDetails(doc, goodreadsId); + if (metadata != null) { + return List.of(metadata); + } } + } catch (Exception e) { + log.warn("GoodReads: ISBN lookup failed: {}, falling back to preview search", e.getMessage()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java` around lines 137 - 148, Wrap the ISBN-path in GoodReadsParser so the call to fetchDoc(BASE_ISBN_URL + isbn) and the subsequent extractGoodreadsIdFromOgUrl/parseBookDetails logic are enclosed in a try-catch that catches RuntimeException (the same way fetchTopMetadata does); on exception log the error (including the ISBN and exception) and allow the method to continue so it falls back to the preview/search flow instead of propagating the RuntimeException. Ensure you reference the existing helper methods extractGoodreadsIdFromOgUrl and parseBookDetails when adding the try-catch so behavior remains identical on success.booklore-api/src/main/java/org/booklore/service/bookdrop/BookdropMetadataService.java (1)
118-127: Maintenance note: Keep synchronized with BookMetadata.The comment on line 119 is helpful. The explicit list is clear and readable, though it does require manual updates when new provider IDs are added to
BookMetadata.For future consideration, an annotation-based approach or a dedicated method on
BookMetadata(e.g.,hasAnyIdentifier()) could reduce the maintenance burden, but this is fine for now.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/service/bookdrop/BookdropMetadataService.java` around lines 118 - 127, Replace the manual list check in hasAnyKnownIdentifier with a delegated method on the BookMetadata model: implement a public boolean hasAnyIdentifier() in BookMetadata that checks all provider ID fields (same list currently used: getIsbn13, getIsbn10, getAsin, getGoodreadsId, getGoogleId, getHardcoverId, getHardcoverBookId, getComicvineId, getDoubanId, getLubimyczytacId, getRanobedbId, getAudibleId) and then update BookdropMetadataService.hasAnyKnownIdentifier(BookMetadata m) to simply return m.hasAnyIdentifier(); this centralizes the identifier-maintenance surface and keeps the comment in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/service/bookdrop/BookdropMetadataService.java`:
- Around line 118-127: Replace the manual list check in hasAnyKnownIdentifier
with a delegated method on the BookMetadata model: implement a public boolean
hasAnyIdentifier() in BookMetadata that checks all provider ID fields (same list
currently used: getIsbn13, getIsbn10, getAsin, getGoodreadsId, getGoogleId,
getHardcoverId, getHardcoverBookId, getComicvineId, getDoubanId,
getLubimyczytacId, getRanobedbId, getAudibleId) and then update
BookdropMetadataService.hasAnyKnownIdentifier(BookMetadata m) to simply return
m.hasAnyIdentifier(); this centralizes the identifier-maintenance surface and
keeps the comment in sync.
In
`@booklore-api/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java`:
- Around line 137-148: Wrap the ISBN-path in GoodReadsParser so the call to
fetchDoc(BASE_ISBN_URL + isbn) and the subsequent
extractGoodreadsIdFromOgUrl/parseBookDetails logic are enclosed in a try-catch
that catches RuntimeException (the same way fetchTopMetadata does); on exception
log the error (including the ISBN and exception) and allow the method to
continue so it falls back to the preview/search flow instead of propagating the
RuntimeException. Ensure you reference the existing helper methods
extractGoodreadsIdFromOgUrl and parseBookDetails when adding the try-catch so
behavior remains identical on success.
In `@CHANGELOG.md`:
- Around line 1-12: Move the top-level heading "# Changelog" to the very first
line of the file so the file begins with that heading, followed by a single
blank line and then the existing release entries (starting with "##
[2.2.2]..."). Ensure the "# Changelog" line is removed from its current location
and placed above the "## [2.2.2]" section so tooling and readability are
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3956752-f3e5-4e24-ad0c-3ef34a067cb8
📒 Files selected for processing (3)
CHANGELOG.mdbooklore-api/src/main/java/org/booklore/service/bookdrop/BookdropMetadataService.javabooklore-api/src/main/java/org/booklore/service/metadata/parser/GoodReadsParser.java
imajes
force-pushed
the
develop
branch
2 times, most recently
from
89113d4 to
37ca101
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
This pull request introduces improvements to metadata fetching and lookup logic for book files, especially around the handling of insufficient metadata and ISBN-based lookups for Goodreads and bookdrop. Normally, you would wanna query via the "most specific" information possible. This, however, with Goodreads parser wasn't the case. It'd premateraly fallback to filenames which were in no-way accurate way to find bookmetata
Linked Issue: Fixes #
🏷️ Type of Change
🔧 Changes
🧪 Testing (MANDATORY)
Manual testing steps you performed:
Regression testing:
Edge cases covered:
Test output:
Backend test output (
./gradlew test)Frontend test output (
ng test)📸 Screen Recording / Screenshots (MANDATORY)
✅ Pre-Submission Checklist
develop(merge conflicts resolved)🤖 AI-Assisted Contributions
TODOs, or unused scaffolding left behind by AI💬 Additional Context (optional)
Summary by CodeRabbit
New Features
Improvements