refactor(nativelib): implement JVM-wide native library availability checks and management#790
refactor(nativelib): implement JVM-wide native library availability checks and management#790balazs-szucs wants to merge 1 commit intogrimmory-tools:developfrom
Conversation
…hecks and management
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/java/org/booklore/nativelib/NativeLibraries.java`:
- Around line 71-77: The catch(Throwable) in runProbe(Probe probe) masks fatal
JVM Errors; modify the handler so fatal Errors propagate: inside the current
catch block (around probe.fn.get()) detect if the thrown Throwable is an Error
that is NOT a LinkageError (i.e., fatal JVM errors like OutOfMemoryError,
StackOverflowError, InternalError, ThreadDeath) and rethrow it, otherwise log
the problem and return false; reference runProbe(Probe probe), probe.fn.get(),
and the existing log.warn(...) call so you update that catch to rethrow fatal
Errors and only treat Exception/LinkageError as native-unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5121cde6-9d5e-44e2-8c40-2bd4bf49437b
📒 Files selected for processing (6)
backend/build.gradle.ktsbackend/src/main/java/org/booklore/nativelib/NativeLibraries.javabackend/src/main/java/org/booklore/nativelib/NativeLibraryManager.javabackend/src/main/java/org/booklore/service/ArchiveService.javabackend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.javabackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
backend/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code
Files:
backend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.javabackend/src/main/java/org/booklore/nativelib/NativeLibraryManager.javabackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.javabackend/src/main/java/org/booklore/service/ArchiveService.javabackend/src/main/java/org/booklore/nativelib/NativeLibraries.java
backend/src/test/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Prefer focused unit tests; use
@SpringBootTestonly when the Spring context is required in backend code
Files:
backend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.javabackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.java
🧠 Learnings (10)
📓 Common learnings
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/src/main/java/org/booklore/service/reader/EpubReaderService.java:402-407
Timestamp: 2026-04-02T09:25:48.330Z
Learning: In grimmory-tools/grimmory, before commenting on any file processing code (epub or pdf), always verify the current state of the relevant upstream grimmory libraries: `grimmory-tools/epub4j` (for epub) and `grimmory-tools/PDFium4j` (for pdf). These custom libraries may have different APIs, capabilities, and limitations compared to the third-party libraries they replace. Issues about streaming, buffering, or API surface may need to be filed/addressed in those upstream repos rather than in grimmory itself.
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/src/main/java/org/booklore/service/reader/EpubReaderService.java:402-407
Timestamp: 2026-04-02T09:25:48.330Z
Learning: In grimmory-tools/grimmory, PDF processing uses `grimmory PDFium4j` from https://github.com/grimmory-tools/PDFium4j. This is a grimmory-maintained library and is fundamentally different from Apache PDFBox. Do NOT generalize API knowledge, behavior, or limitations of PDFBox to grimmory PDFium4j. Before raising review comments on PDF-related file processing code, verify the current API and capabilities in the grimmory-tools/PDFium4j repository.
📚 Learning: 2026-04-02T09:25:48.330Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/src/main/java/org/booklore/service/reader/EpubReaderService.java:402-407
Timestamp: 2026-04-02T09:25:48.330Z
Learning: In grimmory-tools/grimmory, PDF processing uses `grimmory PDFium4j` from https://github.com/grimmory-tools/PDFium4j. This is a grimmory-maintained library and is fundamentally different from Apache PDFBox. Do NOT generalize API knowledge, behavior, or limitations of PDFBox to grimmory PDFium4j. Before raising review comments on PDF-related file processing code, verify the current API and capabilities in the grimmory-tools/PDFium4j repository.
Applied to files:
backend/build.gradle.ktsbackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.java
📚 Learning: 2026-04-14T12:46:59.296Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:46:59.296Z
Learning: In grimmory-tools/grimmory (booklore-api), the team targets Java 25 with `--enable-preview` and actively aims to use the most modern Java conventions available. When reviewing or suggesting code for this project, always prefer modern Java 25 idioms and features (e.g., unnamed variables `_`, records, pattern matching for switch/instanceof, sealed classes, sequenced collections, `Stream.toList()`, text blocks, etc.) over older or more conservative patterns. Do not suggest reverting modern constructs to legacy alternatives.
Applied to files:
backend/build.gradle.kts
📚 Learning: 2026-03-31T06:22:25.311Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 113
File: booklore-api/build.gradle.kts:89-90
Timestamp: 2026-03-31T06:22:25.311Z
Learning: When reviewing build logic or CI checks that call the JitPack build status API, do not treat a response like {"status":"none"} as evidence that a dependency is unavailable or that a build has failed. JitPack typically builds packages on-demand the first time they’re requested (e.g., via Gradle); "none" means the package isn’t pre-built/cached yet. Only raise a build failure concern when the status explicitly indicates an error/failure (e.g., failed/cancelled), or when dependent resolution actually fails.
Applied to files:
backend/build.gradle.kts
📚 Learning: 2026-04-02T09:12:48.158Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/build.gradle.kts:0-0
Timestamp: 2026-04-02T09:12:48.158Z
Learning: In this repo, nightcompress (groupId `com.github.gotson.nightcompress`, artifact `nightcompress`) is the preferred library for archive/unarchive operations (including RAR and other formats). During code review, flag changes to Gradle dependency declarations that remove nightcompress or replace it with an alternative for unarchiving (e.g., junrar, commons-compress, JNI-based libraries, or standard Java `ZipInputStream`/`ZipFile`) unless there’s a justified exception. Exceptions are acceptable only when nightcompress cannot handle a specific archive format; in that case, require the PR to clearly document the format limitation, the alternative being used, and get maintainer confirmation of the trade-off.
Applied to files:
backend/build.gradle.kts
📚 Learning: 2026-04-02T09:25:37.417Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/src/main/java/org/booklore/service/reader/EpubReaderService.java:402-407
Timestamp: 2026-04-02T09:25:37.417Z
Learning: When reviewing epub-related file processing code in this project, remember that grimmory-tools/grimmory uses the `org.grimmory.epub4j` library (from grimmory-tools/epub4j) and that it is a grimmory-maintained fork fundamentally different from `io.documentnode.epub4j`. Do not assume the same API behavior, limitations, or semantics as `io.documentnode.epub4j`. Before flagging issues or proposing changes, verify the current API/capabilities in the grimmory-tools/epub4j repository.
Applied to files:
backend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.java
📚 Learning: 2026-04-10T08:15:37.436Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.
Applied to files:
backend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.javabackend/src/main/java/org/booklore/nativelib/NativeLibraryManager.javabackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.javabackend/src/main/java/org/booklore/service/ArchiveService.javabackend/src/main/java/org/booklore/nativelib/NativeLibraries.java
📚 Learning: 2026-04-14T12:43:08.698Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).
Applied to files:
backend/src/test/java/org/booklore/test/EpubNativeAvailableCondition.javabackend/src/main/java/org/booklore/nativelib/NativeLibraryManager.javabackend/src/test/java/org/booklore/test/PdfiumAvailableCondition.javabackend/src/main/java/org/booklore/service/ArchiveService.javabackend/src/main/java/org/booklore/nativelib/NativeLibraries.java
📚 Learning: 2026-04-02T09:06:44.150Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/build.gradle.kts:94-95
Timestamp: 2026-04-02T09:06:44.150Z
Learning: In `grimmory-tools/grimmory`, the project is committed to `com.github.gotson.nightcompress:nightcompress` for archive/RAR support. Do not suggest replacing it with junrar or any other archive library. `ArchiveService.java` uses nightcompress APIs (`Archive`, `ArchiveEntry`, `LibArchiveException`).
Applied to files:
backend/src/main/java/org/booklore/service/ArchiveService.java
📚 Learning: 2026-04-02T09:12:50.790Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 334
File: booklore-api/build.gradle.kts:0-0
Timestamp: 2026-04-02T09:12:50.790Z
Learning: In `grimmory-tools/grimmory`, nightcompress (`com.github.gotson.nightcompress:nightcompress`) is the preferred library for archive/unarchiving operations (including RAR and other formats). Flag any PR changes that remove or replace nightcompress with an alternative (e.g., junrar, commons-compress, JNI-based libs, or standard Java ZipInputStream/ZipFile) for unarchiving purposes. Nightcompress is considered better than JNI Java alternatives and generally better than standard Java unzip. Exceptions are acceptable when nightcompress cannot handle a specific format, but such trade-offs should be made explicit and confirmed by the maintainer.
Applied to files:
backend/src/main/java/org/booklore/service/ArchiveService.java
🔇 Additional comments (1)
backend/build.gradle.kts (1)
123-125: No action required. The native classifiers forpdfium4j:0.15.0, includingnatives-linux-musl-x64andnatives-linux-musl-arm64, are all published and resolve successfully on Maven Central. Alpine/musl builds will not fail dependency resolution.> Likely an incorrect or invalid review comment.
Description
Linked Issue: Fixes #
Changes
This pull request introduces a centralized, JVM-wide mechanism for probing and managing native library availability in the backend. The main change is the addition of the
NativeLibrariessingleton, which serializes and logs native library loading and provides a single source of truth for native support status. Several services and test utilities are refactored to use this new mechanism, improving reliability and consistency in native library handling.Centralized native library management:
NativeLibrariessingleton inorg.booklore.nativelib, which serializes native library loading, provides availability checks for PDFium, libarchive, and epub4j-native, and logs a summary of the loading status. This ensures native libraries are loaded exactly once and their status is globally accessible.NativeLibraryManageras a Spring-managed delegate overNativeLibrariesfor dependency injection and integration with the application context.Refactoring of native library usage:
ArchiveServiceto useNativeLibrariesfor checking libarchive availability instead of maintaining its own probe logic, and removed related initialization code.Testing:
PdfiumAvailableCondition(used for conditional test execution) to delegate availability checks toNativeLibraries, removing redundant probing logic.EpubNativeAvailableConditionto enable/disable tests based on epub4j-native availability using the centralized loader.Dependency update:
pdfium4jdependency version from0.14.0to0.15.0inbuild.gradle.kts.Summary by CodeRabbit
Release Notes
Chores
Refactor