Merge branch 'main' into grobid-preferences

JabRef · Aug 4, 2024 · dc61c8d · dc61c8d
2 parents 61b29bb + 2c8fc01
commit dc61c8d
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 82 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - The Pubmed/Medline Plain importer now imports the PMID field as well [#11488](https://github.com/JabRef/jabref/issues/11488)
 - The 'Check for updates' menu bar button is now always enabled. [#11485](https://github.com/JabRef/jabref/pull/11485)
 - JabRef respects the [configuration for storing files relative to the .bib file](https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in more cases. [#11492](https://github.com/JabRef/jabref/pull/11492)
+- JabRef does not show finished background tasks in the status bar popup. [#11574](https://github.com/JabRef/jabref/pull/11574)
 - We enhanced the indexing speed. [#11502](https://github.com/JabRef/jabref/pull/11502)
 
 ### Fixed

diff --git a/settings.gradle b/settings.gradle
@@ -19,7 +19,7 @@ pluginManagement {
 }
 
 plugins {
-  id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
+    id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
 }
 
 rootProject.name = "JabRef"
diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java
@@ -6,18 +6,28 @@
     requires java.sql;
     requires java.sql.rowset;
 
-    // JavaFX
+    // region JavaFX
     requires javafx.base;
     requires javafx.graphics;
     requires javafx.controls;
     requires javafx.web;
     requires javafx.fxml;
+
     requires afterburner.fx;
+    provides com.airhacks.afterburner.views.ResourceLocator
+            with org.jabref.gui.util.JabRefResourceLocator;
+
     requires com.dlsc.gemsfx;
     uses com.dlsc.gemsfx.TagsField;
+
+    requires com.tobiasdiez.easybind;
+
     requires de.saxsys.mvvmfx;
-    requires reactfx;
+    requires de.saxsys.mvvmfx.validation;
+
+    requires org.controlsfx.controls;
     requires org.fxmisc.flowless;
+    requires org.fxmisc.richtext;
 
     requires org.kordamp.ikonli.core;
     requires org.kordamp.ikonli.javafx;
@@ -30,20 +40,17 @@
     provides org.kordamp.ikonli.IkonProvider
             with org.jabref.gui.icon.JabrefIconProvider;
 
-    requires org.controlsfx.controls;
-    requires org.fxmisc.richtext;
-    requires com.tobiasdiez.easybind;
-
-    provides com.airhacks.afterburner.views.ResourceLocator
-            with org.jabref.gui.util.JabRefResourceLocator;
+    requires reactfx;
+    // endregion
 
-    // Logging
+    // region: Logging
     requires org.slf4j;
     requires jul.to.slf4j;
     requires org.apache.logging.log4j.to.slf4j;
     requires org.tinylog.api;
     requires org.tinylog.api.slf4j;
     requires org.tinylog.impl;
+    // endregion
 
     provides org.tinylog.writers.Writer
     with org.jabref.gui.logging.GuiWriter;
@@ -55,16 +62,18 @@
     // YAML
     requires org.yaml.snakeyaml;
 
-    // Annotations (@PostConstruct)
+    // region: Annotations (@PostConstruct)
     requires jakarta.annotation;
     requires jakarta.inject;
+    // endregion
 
-    // http server and client exchange
+    // region: http server and client exchange
     requires java.net.http;
     requires jakarta.ws.rs;
     requires org.glassfish.grizzly;
+    // endregion
 
-    // data mapping
+    // region: data mapping
     requires jakarta.xml.bind;
     requires jdk.xml.dom;
     requires com.google.gson;
@@ -73,22 +82,26 @@
     requires com.fasterxml.jackson.datatype.jsr310;
     // needs to be loaded here as it's otherwise not found at runtime
     requires org.glassfish.jaxb.runtime;
+    // endregion
 
     // dependency injection using HK2
     requires org.glassfish.hk2.api;
 
-    // http clients
+    // region: http clients
     requires unirest.java.core;
     requires unirest.modules.gson;
+    requires org.apache.httpcomponents.core5.httpcore5;
     requires org.jsoup;
+    // endregion
 
-    // SQL databases
+    // region: SQL databases
     requires ojdbc10;
     requires org.postgresql.jdbc;
     requires org.mariadb.jdbc;
     uses org.mariadb.jdbc.credential.CredentialPlugin;
+    // endregion
 
-    // Apache Commons and other (similar) helper libraries
+    // region: Apache Commons and other (similar) helper libraries
     requires com.google.common;
     requires io.github.javadiffutils;
     requires java.string.similarity;
@@ -98,9 +111,13 @@
     requires org.apache.commons.lang3;
     requires org.apache.commons.text;
     requires org.apache.commons.logging;
+    // endregion
 
+    // region: latex2unicode
     requires com.github.tomtung.latex2unicode;
     requires fastparse;
+    requires scala.library;
+    // endregion
 
     requires jbibtex;
     requires citeproc.java;
@@ -123,15 +140,15 @@
 
     requires org.jooq.jool;
 
-    // fulltext search
+    // region: fulltext search
     requires org.apache.lucene.core;
     // In case the version is updated, please also adapt SearchFieldConstants#VERSION to the newly used version
     uses org.apache.lucene.codecs.lucene99.Lucene99Codec;
-
     requires org.apache.lucene.queryparser;
     uses org.apache.lucene.queryparser.classic.MultiFieldQueryParser;
     requires org.apache.lucene.analysis.common;
     requires org.apache.lucene.highlighter;
+    // endregion
 
     requires net.harawata.appdirs;
     requires com.sun.jna;
@@ -143,11 +160,10 @@
 
     requires transitive org.jspecify;
 
-    // other libraries
-    requires org.antlr.antlr4.runtime;
-    requires org.libreoffice.uno;
-    requires de.saxsys.mvvmfx.validation;
+    // region: other libraries (alphabetically)
     requires dd.plist;
     requires mslinks;
-    requires org.apache.httpcomponents.core5.httpcore5;
+    requires org.antlr.antlr4.runtime;
+    requires org.libreoffice.uno;
+    // endregion
 }
diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java
@@ -348,7 +348,7 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
     @Override
     public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) {
         TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
-        EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
+        EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks());
         taskProgressView.setRetainTasks(false);
         taskProgressView.setGraphicFactory(BackgroundTask::getIcon);
 

diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java
@@ -16,6 +16,7 @@
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
+import javafx.collections.transformation.FilteredList;
 import javafx.concurrent.Task;
 import javafx.scene.Node;
 import javafx.util.Pair;
@@ -185,8 +186,9 @@ public Optional<Node> getFocusOwner() {
         return focusOwner.get();
     }
 
-    public ObservableList<Task<?>> getBackgroundTasks() {
-        return EasyBind.map(backgroundTasks, Pair::getValue);
+    public ObservableList<Task<?>> getRunningBackgroundTasks() {
+        FilteredList<Pair<BackgroundTask<?>, Task<?>>> pairs = new FilteredList<>(backgroundTasks, task -> task.getValue().isRunning());
+        return EasyBind.map(pairs, Pair::getValue);
     }
 
     public void addBackgroundTask(BackgroundTask<?> backgroundTask, Task<?> task) {

diff --git a/src/main/java/org/jabref/gui/frame/MainToolBar.java b/src/main/java/org/jabref/gui/frame/MainToolBar.java
@@ -45,6 +45,7 @@
 import org.jabref.preferences.PreferencesService;
 
 import com.tobiasdiez.easybind.EasyBind;
+import com.tobiasdiez.easybind.Subscription;
 import org.controlsfx.control.PopOver;
 import org.controlsfx.control.TaskProgressView;
 
@@ -63,6 +64,7 @@ public class MainToolBar extends ToolBar {
 
     private PopOver entryFromIdPopOver;
     private PopOver progressViewPopOver;
+    private Subscription taskProgressSubscription;
 
     public MainToolBar(LibraryTabContainer tabContainer,
                        PushToApplicationCommand pushToApplicationCommand,
@@ -205,15 +207,11 @@ Group createTaskIndicator() {
             }
         });
 
-        /*
-        The label of the indicator cannot be removed with styling. Therefore,
-        hide it and clip it to a square of (width x width) each time width is updated.
-         */
+        // The label of the indicator cannot be removed with styling. Therefore,
+        // hide it and clip it to a square of (width x width) each time width is updated.
         indicator.widthProperty().addListener((observable, oldValue, newValue) -> {
-            /*
-            The indeterminate spinner is wider than the determinate spinner.
-            We must make sure they are the same width for the clipping to result in a square of the same size always.
-             */
+            // The indeterminate spinner is wider than the determinate spinner.
+            // We must make sure they are the same width for the clipping to result in a square of the same size always.
             if (!indicator.isIndeterminate()) {
                 indicator.setPrefWidth(newValue.doubleValue());
             }
@@ -224,23 +222,24 @@ hide it and clip it to a square of (width x width) each time width is updated.
         });
 
         indicator.setOnMouseClicked(event -> {
+            if ((progressViewPopOver != null) && (progressViewPopOver.isShowing())) {
+                progressViewPopOver.hide();
+                taskProgressSubscription.unsubscribe();
+            }
+
             TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
-            EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
-            taskProgressView.setRetainTasks(true);
+            taskProgressSubscription = EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks());
+            taskProgressView.setRetainTasks(false);
             taskProgressView.setGraphicFactory(BackgroundTask::getIcon);
 
             if (progressViewPopOver == null) {
                 progressViewPopOver = new PopOver(taskProgressView);
                 progressViewPopOver.setTitle(Localization.lang("Background Tasks"));
                 progressViewPopOver.setArrowLocation(PopOver.ArrowLocation.RIGHT_TOP);
-                progressViewPopOver.setContentNode(taskProgressView);
-                progressViewPopOver.show(indicator);
-            } else if (progressViewPopOver.isShowing()) {
-                progressViewPopOver.hide();
-            } else {
-                progressViewPopOver.setContentNode(taskProgressView);
-                progressViewPopOver.show(indicator);
             }
+
+            progressViewPopOver.setContentNode(taskProgressView);
+            progressViewPopOver.show(indicator);
         });
 
         return new Group(indicator);

diff --git a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
@@ -128,7 +128,7 @@ public <V> Future<?> schedule(BackgroundTask<V> task, long delay, TimeUnit unit)
     public void shutdown() {
         StateManager stateManager = Injector.instantiateModelOrService(StateManager.class);
         if (stateManager != null) {
-            stateManager.getBackgroundTasks().stream().filter(task -> !task.isDone()).forEach(Task::cancel);
+            stateManager.getRunningBackgroundTasks().stream().forEach(Task::cancel);
         }
         executor.shutdownNow();
         scheduledExecutor.shutdownNow();
@@ -175,9 +175,14 @@ public V call() throws Exception {
             javaTask.setOnRunning(event -> onRunning.run());
         }
         Consumer<V> onSuccess = task.getOnSuccess();
-        if (onSuccess != null) {
-            javaTask.setOnSucceeded(event -> onSuccess.accept(javaTask.getValue()));
-        }
+        javaTask.setOnSucceeded(event -> {
+            // Set to 100% completed on completion
+            task.updateProgress(1, 1);
+
+            if (onSuccess != null) {
+                onSuccess.accept(javaTask.getValue());
+            }
+        });
         Consumer<Exception> onException = task.getOnException();
         if (onException != null) {
             javaTask.setOnFailed(event -> onException.accept(convertToException(javaTask.getException())));

diff --git a/src/main/java/org/jabref/logic/importer/fileformat/CitaviXmlImporter.java b/src/main/java/org/jabref/logic/importer/fileformat/CitaviXmlImporter.java
@@ -1,12 +1,9 @@
 package org.jabref.logic.importer.fileformat;
 
-import java.io.BufferedInputStream;
 import java.io.BufferedReader;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.io.PushbackInputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -55,6 +52,8 @@
 import jakarta.xml.bind.JAXBContext;
 import jakarta.xml.bind.JAXBException;
 import jakarta.xml.bind.Unmarshaller;
+import org.apache.commons.io.ByteOrderMark;
+import org.apache.commons.io.input.BOMInputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -431,7 +430,7 @@ private Object unmarshallRoot(BufferedReader reader) throws XMLStreamException,
     @Override
     public ParserResult importDatabase(BufferedReader reader) throws IOException {
         Objects.requireNonNull(reader);
-        throw new UnsupportedOperationException("CitaviXmlImporter does not support importDatabase(BufferedReader reader)."
+        throw new UnsupportedOperationException("CitaviXmlImporter does not support importDatabase(BufferedReader reader). "
                                                 + "Instead use importDatabase(Path filePath, Charset defaultEncoding).");
     }
 
@@ -447,39 +446,25 @@ public List<BibEntry> parseEntries(InputStream inputStream) {
     }
 
     private BufferedReader getReaderFromZip(Path filePath) throws IOException {
-        ZipInputStream zis = new ZipInputStream(new FileInputStream(filePath.toFile()));
-        ZipEntry zipEntry = zis.getNextEntry();
-
         Path newFile = Files.createTempFile("citavicontent", ".xml");
+        newFile.toFile().deleteOnExit();
 
-        while (zipEntry != null) {
-            Files.copy(zis, newFile, StandardCopyOption.REPLACE_EXISTING);
-
-            zipEntry = zis.getNextEntry();
-        }
-
-        zis.closeEntry();
-
-        InputStream stream = Files.newInputStream(newFile, StandardOpenOption.READ);
-
-        // check and delete the utf-8 BOM bytes
-        InputStream newStream = checkForUtf8BOMAndDiscardIfAny(stream);
-
-        // clean up the temp file
-        Files.delete(newFile);
-
-        return new BufferedReader(new InputStreamReader(newStream, StandardCharsets.UTF_8));
-    }
-
-    private static InputStream checkForUtf8BOMAndDiscardIfAny(InputStream inputStream) throws IOException {
-        PushbackInputStream pushbackInputStream = new PushbackInputStream(new BufferedInputStream(inputStream), 3);
-        byte[] bom = new byte[3];
-        if (pushbackInputStream.read(bom) != -1) {
-            if (!((bom[0] == (byte) 0xEF) && (bom[1] == (byte) 0xBB) && (bom[2] == (byte) 0xBF))) {
-                pushbackInputStream.unread(bom);
+        try (ZipInputStream zis = new ZipInputStream(Files.newInputStream(filePath))) {
+            ZipEntry zipEntry = zis.getNextEntry();
+            while (zipEntry != null) {
+                Files.copy(zis, newFile, StandardCopyOption.REPLACE_EXISTING);
+                zipEntry = zis.getNextEntry();
             }
         }
-        return pushbackInputStream;
+
+        // Citavi XML files sometimes contains BOM markers. We just discard them.
+        // Solution inspired by https://stackoverflow.com/a/37445972/873282
+        return new BufferedReader(
+                new InputStreamReader(
+                        new BOMInputStream(
+                                Files.newInputStream(newFile, StandardOpenOption.READ),
+                                false,
+                                ByteOrderMark.UTF_8, ByteOrderMark.UTF_16BE, ByteOrderMark.UTF_16LE, ByteOrderMark.UTF_32BE, ByteOrderMark.UTF_32LE)));
     }
 
     private String clean(String input) {

diff --git a/src/test/java/org/jabref/logic/importer/fileformat/ImporterTestEngine.java b/src/test/java/org/jabref/logic/importer/fileformat/ImporterTestEngine.java
@@ -52,8 +52,7 @@ public static void testImportEntries(Importer importer, String fileName, String
         if (parserResult.isInvalid()) {
             throw new ImportException(parserResult.getErrorMessage());
         }
-        List<BibEntry> entries = parserResult.getDatabase()
-                                             .getEntries();
+        List<BibEntry> entries = parserResult.getDatabase().getEntries();
         BibEntryAssert.assertEquals(ImporterTestEngine.class, fileName.replaceAll(fileType, ".bib"), entries);
     }