From e34c9efa62a17f2c9e0bfd7b37c2751efda9f145 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sun, 8 Sep 2024 22:27:59 +0200 Subject: [PATCH 1/2] move fallback exception handler to logic and add consumer for thread execution --- .../jabref/gui/FallbackExceptionHandler.java | 28 --------------- src/main/java/org/jabref/gui/JabRefGUI.java | 8 ++++- .../org/jabref/gui/util/UiTaskExecutor.java | 3 +- .../search/indexing/BibFieldsIndexer.java | 2 +- .../logic/util/FallbackExceptionHandler.java | 36 +++++++++++++++++++ .../logic/util/HeadlessExecutorService.java | 5 +-- 6 files changed, 47 insertions(+), 35 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/FallbackExceptionHandler.java create mode 100644 src/main/java/org/jabref/logic/util/FallbackExceptionHandler.java diff --git a/src/main/java/org/jabref/gui/FallbackExceptionHandler.java b/src/main/java/org/jabref/gui/FallbackExceptionHandler.java deleted file mode 100644 index 2f42d2a00a1..00000000000 --- a/src/main/java/org/jabref/gui/FallbackExceptionHandler.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.jabref.gui; - -import org.jabref.gui.util.UiTaskExecutor; - -import com.airhacks.afterburner.injection.Injector; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Catch and log any unhandled exceptions. - */ -public class FallbackExceptionHandler implements Thread.UncaughtExceptionHandler { - - private static final Logger LOGGER = LoggerFactory.getLogger(FallbackExceptionHandler.class); - - public static void installExceptionHandler() { - Thread.setDefaultUncaughtExceptionHandler(new FallbackExceptionHandler()); - } - - @Override - public void uncaughtException(Thread thread, Throwable exception) { - LOGGER.error("Uncaught exception occurred in {}", thread, exception); - UiTaskExecutor.runInJavaFXThread(() -> { - DialogService dialogService = Injector.instantiateModelOrService(DialogService.class); - dialogService.showErrorDialogAndWait("Uncaught exception occurred in " + thread, exception); - }); - } -} diff --git a/src/main/java/org/jabref/gui/JabRefGUI.java b/src/main/java/org/jabref/gui/JabRefGUI.java index 375ee01a4db..0b6add68d8b 100644 --- a/src/main/java/org/jabref/gui/JabRefGUI.java +++ b/src/main/java/org/jabref/gui/JabRefGUI.java @@ -31,6 +31,7 @@ import org.jabref.logic.remote.RemotePreferences; import org.jabref.logic.remote.server.RemoteListenerServerManager; import org.jabref.logic.util.BuildInfo; +import org.jabref.logic.util.FallbackExceptionHandler; import org.jabref.logic.util.HeadlessExecutorService; import org.jabref.logic.util.TaskExecutor; import org.jabref.logic.util.WebViewStore; @@ -85,7 +86,12 @@ public static void setup(List uiCommands, public void start(Stage stage) { this.mainStage = stage; - FallbackExceptionHandler.installExceptionHandler(); + FallbackExceptionHandler.installExceptionHandler((exception, thread) -> { + UiTaskExecutor.runInJavaFXThread(() -> { + DialogService dialogService = Injector.instantiateModelOrService(DialogService.class); + dialogService.showErrorDialogAndWait("Uncaught exception occurred in " + thread, exception); + }); + }); initialize(); diff --git a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java index decf86b9f81..15aeaf0105c 100644 --- a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java +++ b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java @@ -183,7 +183,8 @@ public static Task getJavaFXTask(BackgroundTask task) { } @Override - public V call() throws Exception { + protected V call() throws Exception { + // this requires that background task call is public as it's in another package return task.call(); } }; diff --git a/src/main/java/org/jabref/logic/search/indexing/BibFieldsIndexer.java b/src/main/java/org/jabref/logic/search/indexing/BibFieldsIndexer.java index de8aa440800..25612b01de1 100644 --- a/src/main/java/org/jabref/logic/search/indexing/BibFieldsIndexer.java +++ b/src/main/java/org/jabref/logic/search/indexing/BibFieldsIndexer.java @@ -36,7 +36,7 @@ public class BibFieldsIndexer implements LuceneIndexer { public BibFieldsIndexer(BibDatabaseContext databaseContext) { this.databaseContext = databaseContext; - this.libraryName = databaseContext.getDatabasePath().map(path -> path.getFileName().toString()).orElseGet(() -> "unsaved"); + this.libraryName = databaseContext.getDatabasePath().map(path -> path.getFileName().toString()).orElse("unsaved"); IndexWriterConfig config = new IndexWriterConfig(SearchFieldConstants.LATEX_AWARE_NGRAM_ANALYZER); diff --git a/src/main/java/org/jabref/logic/util/FallbackExceptionHandler.java b/src/main/java/org/jabref/logic/util/FallbackExceptionHandler.java new file mode 100644 index 00000000000..9e0d31539ef --- /dev/null +++ b/src/main/java/org/jabref/logic/util/FallbackExceptionHandler.java @@ -0,0 +1,36 @@ +package org.jabref.logic.util; + +import java.util.function.BiConsumer; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Catch and log any unhandled exceptions. + */ +public class FallbackExceptionHandler implements Thread.UncaughtExceptionHandler { + + private static final Logger LOGGER = LoggerFactory.getLogger(FallbackExceptionHandler.class); + + private final BiConsumer onException; + + public FallbackExceptionHandler(BiConsumer onException) { + this.onException = onException; + } + + public FallbackExceptionHandler() { + this(null); + } + + public static void installExceptionHandler(BiConsumer onException) { + Thread.setDefaultUncaughtExceptionHandler(new FallbackExceptionHandler(onException)); + } + + @Override + public void uncaughtException(Thread thread, Throwable exception) { + LOGGER.error("Uncaught exception occurred in {}", thread, exception); + if (this.onException != null) { + this.onException.accept(exception, thread); + } + } +} diff --git a/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java b/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java index 41589319d0f..1070b29dfe2 100644 --- a/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java +++ b/src/main/java/org/jabref/logic/util/HeadlessExecutorService.java @@ -14,16 +14,13 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import org.jabref.gui.FallbackExceptionHandler; -import org.jabref.gui.util.UiTaskExecutor; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Responsible for managing of all threads (except GUI threads) in JabRef. *

- * GUI background tasks should run in {@link UiTaskExecutor}. + * GUI background tasks should run in {@link org.jabref.gui.util.UiTaskExecutor}. *

* This is a wrapper around {@link ExecutorService} *

From a3a354a6003c8d3866ab9741ad1f6b05135349ee Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sun, 8 Sep 2024 22:42:31 +0200 Subject: [PATCH 2/2] remove dialog service from logic class --- .../DuplicateResolverDialog.java | 2 -- .../gui/linkedfile/DownloadLinkedFileAction.java | 2 +- .../jabref/logic/util/io/FileNameUniqueness.java | 11 +++++------ .../logic/util/io/FileNameUniquenessTest.java | 16 ++++++++++------ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java index 56e32157125..939351254d5 100644 --- a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java +++ b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java @@ -23,7 +23,6 @@ public class DuplicateResolverDialog extends BaseDialog { - private final BibDatabaseContext database; private final StateManager stateManager; public enum DuplicateResolverType { @@ -72,7 +71,6 @@ public DuplicateResolverDialog(BibEntry one, DialogService dialogService, PreferencesService preferencesService) { this.setTitle(Localization.lang("Possible duplicate entries")); - this.database = database; this.stateManager = stateManager; this.dialogService = dialogService; this.preferencesService = preferencesService; diff --git a/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java b/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java index 355b7da7097..d02f9faf85d 100644 --- a/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java +++ b/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java @@ -148,7 +148,7 @@ private void onSuccess(Path targetDirectory, Path downloadedFile) { boolean isDuplicate; boolean isHtml; try { - isDuplicate = FileNameUniqueness.isDuplicatedFile(targetDirectory, downloadedFile.getFileName(), dialogService); + isDuplicate = FileNameUniqueness.isDuplicatedFile(targetDirectory, downloadedFile.getFileName(), dialogService::notify); } catch (IOException e) { LOGGER.error("FileNameUniqueness.isDuplicatedFile failed", e); return; diff --git a/src/main/java/org/jabref/logic/util/io/FileNameUniqueness.java b/src/main/java/org/jabref/logic/util/io/FileNameUniqueness.java index 44a73e0ae71..1b98276dab2 100644 --- a/src/main/java/org/jabref/logic/util/io/FileNameUniqueness.java +++ b/src/main/java/org/jabref/logic/util/io/FileNameUniqueness.java @@ -5,10 +5,10 @@ import java.nio.file.Path; import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.jabref.gui.DialogService; import org.jabref.logic.l10n.Localization; import org.slf4j.Logger; @@ -60,18 +60,17 @@ public static String getNonOverWritingFileName(Path targetDirectory, String file * * @param directory The directory which saves the files (.pdf, for example) * @param fileName Suggest name for the newly downloaded file - * @param dialogService To display the error and success message + * @param messageOnDeletion To display the error and success message * @return true when the content of the newly downloaded file is same as the file with "similar" name, * false when there is no "similar" file name or the content is different from that of files with "similar" name * @throws IOException Fail when the file is not exist or something wrong when reading the file */ - public static boolean isDuplicatedFile(Path directory, Path fileName, DialogService dialogService) throws IOException { + public static boolean isDuplicatedFile(Path directory, Path fileName, Consumer messageOnDeletion ) throws IOException { Objects.requireNonNull(directory); Objects.requireNonNull(fileName); - Objects.requireNonNull(dialogService); String extensionSuffix = FileUtil.getFileExtension(fileName).orElse(""); - extensionSuffix = "".equals(extensionSuffix) ? extensionSuffix : "." + extensionSuffix; + extensionSuffix = extensionSuffix.isEmpty() ? extensionSuffix : "." + extensionSuffix; String newFilename = FileUtil.getBaseName(fileName) + extensionSuffix; String fileNameWithoutDuplicated = eraseDuplicateMarks(FileUtil.getBaseName(fileName)); @@ -89,7 +88,7 @@ public static boolean isDuplicatedFile(Path directory, Path fileName, DialogServ if (com.google.common.io.Files.equal(originalFile.toFile(), duplicateFile.toFile())) { try { Files.delete(duplicateFile); - dialogService.notify(Localization.lang("File '%1' is a duplicate of '%0'. Keeping '%0'", originalFileName, fileName)); + messageOnDeletion.accept(Localization.lang("File '%1' is a duplicate of '%0'. Keeping '%0'", originalFileName, fileName)); } catch (IOException e) { LOGGER.error("File '{}' is a duplicate of '{}'. Could not delete '{}'.", fileName, originalFileName, fileName); } diff --git a/src/test/java/org/jabref/logic/util/io/FileNameUniquenessTest.java b/src/test/java/org/jabref/logic/util/io/FileNameUniquenessTest.java index a0035e52a4a..4dfd5d51fb3 100644 --- a/src/test/java/org/jabref/logic/util/io/FileNameUniquenessTest.java +++ b/src/test/java/org/jabref/logic/util/io/FileNameUniquenessTest.java @@ -5,9 +5,12 @@ import java.nio.file.Path; import org.jabref.gui.DialogService; +import org.jabref.gui.push.PushToTexShop; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -16,6 +19,8 @@ class FileNameUniquenessTest { + private static final Logger LOGGER = LoggerFactory.getLogger(FileNameUniquenessTest.class); + @TempDir protected Path tempDir; @@ -56,13 +61,12 @@ void isDuplicatedFileWithNoSimilarNames() throws IOException { Path filePath1 = tempDir.resolve(filename1); Files.createFile(filePath1); - boolean isDuplicate = FileNameUniqueness.isDuplicatedFile(tempDir, filePath1, dialogService); + boolean isDuplicate = FileNameUniqueness.isDuplicatedFile(tempDir, filePath1, LOGGER::info); assertFalse(isDuplicate); } @Test void isDuplicatedFileWithOneSimilarNames() throws IOException { - DialogService dialogService = mock(DialogService.class); String filename1 = "file.txt"; String filename2 = "file (1).txt"; Path filePath1 = tempDir.resolve(filename1); @@ -70,7 +74,7 @@ void isDuplicatedFileWithOneSimilarNames() throws IOException { Files.createFile(filePath1); Files.createFile(filePath2); - boolean isDuplicate = FileNameUniqueness.isDuplicatedFile(tempDir, filePath2, dialogService); + boolean isDuplicate = FileNameUniqueness.isDuplicatedFile(tempDir, filePath2, LOGGER::info); assertTrue(isDuplicate); } @@ -82,21 +86,21 @@ void taseDuplicateMarksReturnsOrignalFileName1() throws IOException { } @Test - void taseDuplicateMarksReturnsOrignalFileName2() throws IOException { + void taseDuplicateMarksReturnsOrignalFileName2() { String fileName1 = "abc (def) gh (1)"; String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1); assertEquals("abc (def) gh", fileName2); } @Test - void taseDuplicateMarksReturnsSameName1() throws IOException { + void taseDuplicateMarksReturnsSameName1() { String fileName1 = "abc def (g)"; String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1); assertEquals("abc def (g)", fileName2); } @Test - void taseDuplicateMarksReturnsSameName2() throws IOException { + void taseDuplicateMarksReturnsSameName2() { String fileName1 = "abc def"; String fileName2 = FileNameUniqueness.eraseDuplicateMarks(fileName1); assertEquals("abc def", fileName2);