OCR integration #13313

Kaan0029 · 2025-06-12T19:01:52Z

In about one to three sentences, describe the changes you have made: what, where, why, ...

Steps to test

Describe how reviewers can test this fix/feature. Ideally, think of how you would guide a beginner user of Jabef to try out your change.
You can add screenshots or videos (using Loom or by just adding .mp4 files).

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[.] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[.] Tests created for changes (if applicable)
[.] Manually tested changed features in running JabRef (always required)
[.] Screenshots added in PR description (if change is visible to the user)
[.] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[.] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

jabref-machine · 2025-06-12T19:06:15Z

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

calixtus · 2025-06-12T19:15:56Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

+        File pdfFile = pdfPath.toFile();
+        if (!pdfFile.exists()) {
+            throw new OcrException("PDF file does not exist: " + pdfPath);
+        }


Use exceptions only for exceptional states. See https://github.com/HugoMatilla/Effective-JAVA-Summary?tab=readme-ov-file#57-use-exceptions-only-for-exceptional-conditions

jabref-machine · 2025-06-12T19:18:50Z

JUnit tests of jabsrv are failing. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Unit tests (pull_request)" and click on it.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Siedlerchr · 2025-06-12T19:35:24Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

+     * Currently uses Tesseract with English language support.
+     */
+    public OcrService() {
+        if (Platform.isMac()) {


Adapted from https://github.com/nguyenq/tess4j/pull/240/files

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

subhramit · 2025-06-12T22:22:35Z

jablib/src/main/java/org/jabref/logic/ocr/OcrException.java

+ * This exception wraps lower-level OCR engine exceptions to provide
+ * a consistent interface for error handling throughout JabRef.
+ */
+public class OcrException extends Exception {


I like the idea, only drawback I see is this may be hard to maintain/stay consistent with as the project grows or if external contributors wish to add something...

Best is to avoid exceptions being thrown at all, especially for expected possible stupid user behaviour. And if they are being thrown, keep them as informative as possible. The consistent interface is Exception at the end either way.

Backstory: This was based on my experience in OO - all the JStyle classes and the OO GUI class had a custom error wrapper, OOError. There was also OOResult and OOVoidResult. I still don't know much about them, but for OOError, apart from just wrapping exceptions, it also acted as an interface for with localizing and displaying error messages. As obvious, when I was new, I found these hard to use. So I started by using native exceptions as thrown by the library methods during the CSL project.
It was convenient, and it worked, and people were fine with the inconsistency, so now 50% of OO uses that, 50% doesn't. I mentioned this as some free time refactoring in #11829, but never had the energy to change it.

trag-bot · 2025-06-19T18:42:04Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    public static OcrResult success(String text) {
+        return new OcrResult(true, text, null);
+    }


Factory method passes null explicitly as a parameter. This violates the principle of not passing null to methods. Consider using Optional or restructuring to avoid null parameter.

trag-bot · 2025-06-19T18:42:05Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    public static OcrResult failure(String errorMessage) {
+        return new OcrResult(false, null, errorMessage);
+    }


Factory method passes null explicitly as a parameter. This violates the principle of not passing null to methods. Consider using Optional or restructuring to avoid null parameter.

Optional should only be a return type

trag-bot · 2025-06-19T18:43:00Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

+    // The OCR engine instance
+    private final Tesseract tesseract;


The comment is trivial and can be derived directly from the code. It doesn't add any new information about the implementation or reasoning behind using Tesseract.

Siedlerchr · 2025-06-19T18:49:58Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

-        // For now, we'll use a relative path that works during development
-        tesseract.setDatapath("tessdata");
+        for (String path : possiblePaths) {
+            File tessdata = new File(path, "tessdata");


use java.nio with Path and File.exists(...) https://docs.oracle.com/javase/tutorial/essential/io/legacy.html

Siedlerchr · 2025-06-19T18:53:03Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    private final String text;
+    private final String errorMessage;
+
+    private OcrResult(boolean success, String text, String errorMessage) {


Maybe can be converted to a record ? https://docs.oracle.com/en/java/javase/24/language/records.html

Siedlerchr · 2025-06-19T19:00:14Z

jabgui/src/main/java/org/jabref/gui/linkedfile/OcrAction.java

+                                  Localization.lang("OCR failed"),
+                                  exception.getMessage()
+                          );
+                      })


before add
.showToUser(true) then it will be shown in the UI task list

subhramit · 2025-06-19T19:03:56Z

jablib/src/main/java/org/jabref/logic/ocr/OcrResult.java

+    private OcrResult(boolean success, String text, String errorMessage) {
+        this.success = success;
+        this.text = text;
+        this.errorMessage = errorMessage;
+    }
+
+    public static OcrResult success(String text) {
+        return new OcrResult(true, text, null);


There should be a better way to think about the constructor parameters. Maybe have different constructors to avoid passing null (as both success and failure are very expected cases)?

The other idea is to model a sealed class hierarchy with sucess/failure as simple record and then you can do pattern matching with switch ...
https://medium.com/@sandip.v.salunkhe/java-record-and-sealed-classes-features-to-enhance-modelling-data-35705c571f70

example at the bottom https://gamlor.info/posts-output/2022-06-01-java-advanced-enums/en/

subhramit · 2025-06-19T19:27:32Z

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java

+        } else {
+            throw new OcrException("Could not find tessdata directory. Please set TESSDATA_PREFIX environment variable.");


Avoid logical branching to throw exceptions.
Can this be modeled with try-catch?
If not, and if this is an expected case, there should just be an error dialog shown.

Kaan0029 and others added 2 commits June 12, 2025 20:57

Initial implementation using tess4j

a51e3b0

merge

aca504a

calixtus changed the title ~~Initial implementation using tess4j~~ OCR integration Jun 12, 2025

calixtus added component: external-files project: gsoc component: pdf-viewer labels Jun 12, 2025

calixtus reviewed Jun 12, 2025

View reviewed changes

add brew to jna path

48ffb06

Siedlerchr reviewed Jun 12, 2025

View reviewed changes

subhramit reviewed Jun 12, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/ocr/OcrService.java Outdated Show resolved Hide resolved

subhramit reviewed Jun 12, 2025

View reviewed changes

subhramit added the component: ocr label Jun 12, 2025

Adapted Exception handling and configured tessdata variable

5a256ae

trag-bot bot reviewed Jun 19, 2025

View reviewed changes

Siedlerchr reviewed Jun 19, 2025

View reviewed changes

subhramit reviewed Jun 19, 2025

View reviewed changes

		// The OCR engine instance
		private final Tesseract tesseract;

		} else {
		throw new OcrException("Could not find tessdata directory. Please set TESSDATA_PREFIX environment variable.");

Uh oh!

OCR integration #13313

Are you sure you want to change the base?

OCR integration #13313

Uh oh!

Conversation

Kaan0029 commented Jun 12, 2025

Steps to test

Mandatory checks

Uh oh!

jabref-machine commented Jun 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jabref-machine commented Jun 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

subhramit Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Siedlerchr Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subhramit Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

subhramit Jun 12, 2025 •

edited

Loading

subhramit Jun 13, 2025 •

edited

Loading

Siedlerchr Jun 19, 2025 •

edited

Loading

subhramit Jun 19, 2025 •

edited

Loading