refactor: update PDFium library version to 0.15.0 and improve native library loading with system property support#33
Conversation
…library loading with system property support
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion bump from 0.14.0 to 0.15.0 accompanied by refactoring of PDFium native library initialization using an initialization-on-demand holder pattern, plus new public APIs for availability checks and custom library path support via system properties. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/grimmory/pdfium4j/internal/NativeLoader.java`:
- Around line 42-64: The override-path branch using
System.getProperty(PROP_LIBRARY_PATH) currently catches UnsatisfiedLinkError,
wraps it in NativeLoadException and assigns loadError but then returns to the
outer try/catch where the outer catch (NativeLoadException) allows fall-through
to System.loadLibrary("pdfium"); change the logic so that when an override path
was attempted you rethrow the NativeLoadException (or propagate it) instead of
letting outer catch fall back: add a boolean flag (e.g., overrideAttempted) set
when PROP_LIBRARY_PATH is non-null, set loadError and throw the
NativeLoadException immediately on failure in the override block, and in the
outer catch check overrideAttempted (or detect loadError from override) and
rethrow the original NativeLoadException rather than calling
System.loadLibrary("pdfium"); also add a unit test that sets
pdfium4j.library.path to a nonexistent absolute path and asserts the thrown
error is the override failure (from NativeLoadException) and no system-library
fallback occurs.
In `@src/main/java/org/grimmory/pdfium4j/PdfiumLibrary.java`:
- Around line 60-62: The initialize() method currently dereferences Holder.STATE
which forces State construction and native init even when
pdfium4j.autoinit=false; change PdfiumLibrary.initialize() to first read the
"pdfium4j.autoinit" system/property flag and only call Holder.STATE.require()
when autoinit is true. Also audit any other places that access Holder.STATE (the
block around the other referenced code at lines ~95-104) and replace direct
static dereferences with conditional checks or a safe accessor so that
Holder.STATE is not constructed unless autoinit is enabled or an explicit
initialize() was requested.
- Around line 86-93: The loadError() accessor must not trigger Holder.STATE
initialization; introduce a private static volatile Throwable field (e.g.
publishedLoadError) on PdfiumLibrary and change loadError() to return that field
directly instead of referencing Holder.STATE. Update the Holder.STATE/State
construction code so that when initialization runs it catches and stores any
Throwable into PdfiumLibrary.publishedLoadError (publishing failures) without
requiring callers to access Holder.STATE, and ensure State's constructor does
not rely on lazy accessors that would be invoked by loadError().
🪄 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: 3505e43e-2baa-496f-8c3e-b92e7c091534
📒 Files selected for processing (3)
build.gradle.ktssrc/main/java/org/grimmory/pdfium4j/PdfiumLibrary.javasrc/main/java/org/grimmory/pdfium4j/internal/NativeLoader.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
🪛 PMD (7.23.0)
src/main/java/org/grimmory/pdfium4j/internal/NativeLoader.java
[Medium] 46-46: The value assigned to field 'loaded' is never used (overwritten on line 67) (UnusedAssignment)
(Best Practices)
🔇 Additional comments (1)
build.gradle.kts (1)
16-16: Backward compatibility confirmed for the minor version bump.The version bump from 0.14.0 to 0.15.0 maintains backward compatibility. Verification confirms that all public APIs—including
PdfDocument.probe(),PdfDocument.koReaderPartialMd5(), andKoReaderChecksum.calculate()methods—remain intact and unchanged. No public methods were removed or modified in breaking ways, and no deprecation annotations were added. The minor version increment is appropriate for this release.
…der and PdfiumLibrary
1 participant
Summary by CodeRabbit
Release Notes v0.15.0
New Features
isAvailable()method to check if PDFium initialized successfullyloadError()method to retrieve initialization failure detailsBug Fixes
Chores