-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
OCR integration #13313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
OCR integration #13313
Changes from 5 commits
a51e3b0
aca504a
48ffb06
5a256ae
f80cec8
db1f577
4020d3a
4987978
9842dd4
8b133e6
42704ea
a64e1ea
6069bc1
5734252
ab98a3a
62dce25
0949300
e4e45f3
15272a6
14332af
5488deb
26a8500
e0da447
f1a06ac
42d34ca
3ffa1c1
59d87a2
bc36a94
e7a043d
caa48f1
9b51b23
c115803
f16c1f4
8c51016
5843c69
1bd2e5b
c7fcc6b
a8bab25
9b9c7b7
edc0343
695e2b4
e5e651e
9ffdbff
facb463
265a312
e4d16c2
cd52669
2eb2df2
809e5a1
d0a20e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
import javafx.collections.ObservableList; | ||
import javafx.scene.control.ContextMenu; | ||
import javafx.scene.control.MenuItem; | ||
import javafx.scene.control.SeparatorMenuItem; | ||
|
||
import org.jabref.gui.DialogService; | ||
|
@@ -10,7 +11,10 @@ | |
import org.jabref.gui.copyfiles.CopySingleFileAction; | ||
import org.jabref.gui.fieldeditors.LinkedFileViewModel; | ||
import org.jabref.gui.fieldeditors.LinkedFilesEditorViewModel; | ||
import org.jabref.gui.linkedfile.OcrAction; | ||
import org.jabref.gui.preferences.GuiPreferences; | ||
import org.jabref.logic.l10n.Localization; | ||
import org.jabref.logic.util.TaskExecutor; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.entry.BibEntry; | ||
|
||
|
@@ -25,21 +29,24 @@ public class ContextMenuFactory { | |
private final LinkedFilesEditorViewModel viewModel; | ||
private final SingleContextCommandFactory singleCommandFactory; | ||
private final MultiContextCommandFactory multiCommandFactory; | ||
private final TaskExecutor taskExecutor; | ||
|
||
public ContextMenuFactory(DialogService dialogService, | ||
GuiPreferences preferences, | ||
BibDatabaseContext databaseContext, | ||
ObservableOptionalValue<BibEntry> bibEntry, | ||
LinkedFilesEditorViewModel viewModel, | ||
SingleContextCommandFactory singleCommandFactory, | ||
MultiContextCommandFactory multiCommandFactory) { | ||
MultiContextCommandFactory multiCommandFactory, | ||
TaskExecutor taskExecutor) { | ||
this.dialogService = dialogService; | ||
this.preferences = preferences; | ||
this.databaseContext = databaseContext; | ||
this.bibEntry = bibEntry; | ||
this.viewModel = viewModel; | ||
this.singleCommandFactory = singleCommandFactory; | ||
this.multiCommandFactory = multiCommandFactory; | ||
this.taskExecutor = taskExecutor; | ||
} | ||
|
||
public ContextMenu createForSelection(ObservableList<LinkedFileViewModel> selectedFiles) { | ||
|
@@ -86,9 +93,45 @@ private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) { | |
factory.createMenuItem(StandardActions.DELETE_FILE, singleCommandFactory.build(StandardActions.DELETE_FILE, linkedFile)) | ||
); | ||
|
||
// Add OCR menu item for PDF files | ||
if (linkedFile.getFile().getFileType().equalsIgnoreCase("pdf")) { | ||
menu.getItems().add(new SeparatorMenuItem()); | ||
|
||
MenuItem ocrItem = createOcrMenuItem(linkedFile); | ||
menu.getItems().add(ocrItem); | ||
} | ||
|
||
return menu; | ||
} | ||
|
||
/** | ||
* Creates the OCR menu item for a PDF file. | ||
* The menu item is only enabled if the PDF file exists on disk. | ||
* | ||
* @param linkedFile The linked PDF file | ||
* @return MenuItem configured for OCR action | ||
*/ | ||
private MenuItem createOcrMenuItem(LinkedFileViewModel linkedFile) { | ||
MenuItem ocrItem = new MenuItem(Localization.lang("Extract text (OCR)")); | ||
|
||
// Create the OCR action | ||
OcrAction ocrAction = new OcrAction( | ||
Comment on lines
+121
to
+122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is trivial and does not add any new information beyond what is clearly visible in the code. It simply restates what the code is doing. |
||
linkedFile.getFile(), | ||
databaseContext, | ||
dialogService, | ||
preferences.getFilePreferences(), | ||
taskExecutor | ||
); | ||
|
||
// Set the action to execute when clicked | ||
ocrItem.setOnAction(event -> ocrAction.execute()); | ||
Comment on lines
+131
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is redundant and merely describes what the code does without providing additional context or reasoning. The code is self-explanatory. |
||
|
||
// Disable if the action is not executable (file doesn't exist) | ||
ocrItem.disableProperty().bind(ocrAction.executableProperty().not()); | ||
Comment on lines
+134
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment restates what is evident from the code itself without providing additional insight or explanation about the underlying logic or design decision. |
||
|
||
return ocrItem; | ||
} | ||
|
||
@FunctionalInterface | ||
public interface SingleContextCommandFactory { | ||
ContextAction build(StandardActions action, LinkedFileViewModel file); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
package org.jabref.gui.linkedfile; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.gui.StateManager; | ||
import org.jabref.gui.actions.Action; | ||
import org.jabref.gui.actions.ActionHelper; | ||
import org.jabref.gui.actions.SimpleCommand; | ||
import org.jabref.logic.util.BackgroundTask; | ||
import org.jabref.logic.util.TaskExecutor; | ||
import org.jabref.logic.l10n.Localization; | ||
import org.jabref.logic.ocr.OcrService; | ||
import org.jabref.logic.ocr.OcrResult; | ||
import org.jabref.logic.ocr.OcrException; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.entry.LinkedFile; | ||
import org.jabref.logic.FilePreferences; | ||
|
||
import java.nio.file.Path; | ||
import java.util.Optional; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
/** | ||
* Action for performing OCR (Optical Character Recognition) on linked PDF files. | ||
*/ | ||
public class OcrAction extends SimpleCommand { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(OcrAction.class); | ||
|
||
private final LinkedFile linkedFile; | ||
private final BibDatabaseContext databaseContext; | ||
private final DialogService dialogService; | ||
private final FilePreferences filePreferences; | ||
private final TaskExecutor taskExecutor; | ||
|
||
public OcrAction(LinkedFile linkedFile, | ||
BibDatabaseContext databaseContext, | ||
DialogService dialogService, | ||
FilePreferences filePreferences, | ||
TaskExecutor taskExecutor) { | ||
this.linkedFile = linkedFile; | ||
this.databaseContext = databaseContext; | ||
this.dialogService = dialogService; | ||
this.filePreferences = filePreferences; | ||
this.taskExecutor = taskExecutor; | ||
|
||
// Only executable for existing PDF files | ||
this.executable.set( | ||
linkedFile.getFileType().equalsIgnoreCase("pdf") && | ||
linkedFile.findIn(databaseContext, filePreferences).isPresent() | ||
); | ||
} | ||
|
||
@Override | ||
public void execute() { | ||
Optional<Path> filePath = linkedFile.findIn(databaseContext, filePreferences); | ||
|
||
if (filePath.isEmpty()) { | ||
dialogService.showErrorDialogAndWait( | ||
Localization.lang("File not found"), | ||
Localization.lang("Could not locate the PDF file on disk.") | ||
); | ||
return; | ||
} | ||
|
||
dialogService.notify(Localization.lang("Performing OCR...")); | ||
|
||
BackgroundTask<OcrResult> task = BackgroundTask.wrap(() -> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, it would be very great if you could make this background task as a separate class (just inherit from |
||
try { | ||
OcrService ocrService = new OcrService(); | ||
return ocrService.performOcr(filePath.get()); | ||
} catch (OcrException e) { | ||
// This is a bug in JabRef | ||
LOGGER.error("Failed to initialize OCR service", e); | ||
return OcrResult.failure("OCR service initialization failed: " + e.getMessage()); | ||
} | ||
}) | ||
.showToUser(true) // Show in task list | ||
.withInitialMessage(Localization.lang("Performing OCR on %0", linkedFile.getLink())); | ||
|
||
task.onSuccess(result -> { | ||
// Use pattern matching with the sealed class | ||
switch (result) { | ||
case OcrResult.Success success -> { | ||
String extractedText = success.text(); | ||
if (extractedText.isEmpty()) { | ||
dialogService.showInformationDialogAndWait( | ||
Localization.lang("OCR Complete"), | ||
Localization.lang("No text was found in the PDF.") | ||
); | ||
} else { | ||
// Show preview | ||
String preview = extractedText.length() > 1000 | ||
? extractedText.substring(0, 1000) + "..." | ||
: extractedText; | ||
|
||
dialogService.showInformationDialogAndWait( | ||
Localization.lang("OCR Result"), | ||
preview | ||
); | ||
} | ||
} | ||
case OcrResult.Failure failure -> { | ||
dialogService.showErrorDialogAndWait( | ||
Localization.lang("OCR failed"), | ||
failure.errorMessage() | ||
); | ||
} | ||
} | ||
}) | ||
.onFailure(exception -> { | ||
dialogService.showErrorDialogAndWait( | ||
Localization.lang("OCR failed"), | ||
exception.getMessage() | ||
); | ||
}) | ||
.executeWith(taskExecutor); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package org.jabref.logic.ocr; | ||
|
||
/** | ||
* Exception thrown when OCR operations fail. | ||
* This exception wraps lower-level OCR engine exceptions to provide | ||
* a consistent interface for error handling throughout JabRef. | ||
*/ | ||
public class OcrException extends Exception { | ||
Kaan0029 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would recommend not adding Try not to throw even JabRefExceptions. Try to return Optional or Results (you have OcrResult - which is great!). And if some internal library could throw an exception - just add it in the method declaration There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and catch errors in upper code. As I seen in the conversation above other mentors has aslo explained this approach There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
100% agree with this |
||
|
||
/** | ||
* Constructs an OcrException with a message and underlying cause. | ||
* | ||
* @param message Descriptive error message | ||
* @param cause The underlying exception that caused this error | ||
*/ | ||
public OcrException(String message, Throwable cause) { | ||
super(message, cause); | ||
} | ||
|
||
/** | ||
* Constructs an OcrException with only a message. | ||
* | ||
* @param message Descriptive error message | ||
*/ | ||
public OcrException(String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package org.jabref.logic.ocr; | ||
|
||
/** | ||
* Represents the result of an OCR operation. | ||
* Uses sealed classes to ensure type safety and avoid null parameters. | ||
*/ | ||
public sealed interface OcrResult { | ||
|
||
/** | ||
* Represents a successful OCR operation with extracted text. | ||
*/ | ||
record Success(String text) implements OcrResult { | ||
public Success { | ||
// Convert null to empty string instead of throwing exception | ||
Kaan0029 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (text == null) { | ||
text = ""; | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Represents a failed OCR operation with an error message. | ||
*/ | ||
record Failure(String errorMessage) implements OcrResult { | ||
public Failure { | ||
// Provide default message instead of throwing exception | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is trivial and simply describes what the code does. The code is self-explanatory and doesn't need this comment. |
||
if (errorMessage == null || errorMessage.isBlank()) { | ||
Kaan0029 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
errorMessage = "Unknown error occurred"; | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Checks if this result represents a success. | ||
*/ | ||
default boolean isSuccess() { | ||
return this instanceof Success; | ||
} | ||
|
||
/** | ||
* Checks if this result represents a failure. | ||
*/ | ||
default boolean isFailure() { | ||
return this instanceof Failure; | ||
} | ||
|
||
/** | ||
* Factory method for creating a success result. | ||
*/ | ||
static OcrResult success(String text) { | ||
return new Success(text); | ||
} | ||
|
||
/** | ||
* Factory method for creating a failure result. | ||
*/ | ||
static OcrResult failure(String errorMessage) { | ||
return new Failure(errorMessage); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.