Add FileMonitor for LaTeX citations

[VinterSallad]

Attempts to implement solution so that Latex Citations tab uses file monitoring to refresh itself instead of having to manually refresh after changes to files in the directory.

So far adds DefaultFileUpdateMonitor to the logic for Latex Citation tab and is now therefore able to refresh itself whenever a change happens in the targeted directory.

Closes #10585

• Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Screenshots added in PR description (for UI changes)
• 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.

[VinterSallad]

Known issues:

• file monitoring persists even after changing the targeted directory, so if a change happens in the old directory, the tab still refreshes
• not able to track changes in nested directories (a change in a nested directory will not trigger the refresh)
• no shutdown (as requested in the issue)

[calixtus]

Hi, thanks for your interest in JabRef and OpenSource. Thank you especially for creating a PR early, so we can read you changes early and comment, if we see something going in a wrong direction. Which may be the case here. I spotted a small problem in your changes to DefaultFileUpdateMonitor. A solution that comes to mind spontaneously may be to inject a runner into the method, which may be in the ui thread?

[koppor]

Small comments.

Did you try it out? I checked org.jabref.gui.util.DefaultFileUpdateMonitor#addListenerForFile, which is code for a file not a directory.

The idea of the issue was that

• all .tex files are collected
• a watcher for each tex file is instantiated

Optionally: In case a user adds a new file to the directory, that file is monitored.

---

Implementation hint:

Class org.jabref.logic.texparser.DefaultLatexParser needs to be modified to know the found latex citation keys per latex file. In case an "udpate" of the informaiton is triggered (by the file update monitor!!), all existing keys should be dropped and readded. Thus, org.jabref.model.texparser.LatexParserResult also needs to adapted.

Test cases need to be written.

[koppor]

Only a comment. I tried to think whether com.airhacks.afterburner.injection.Injector#instantiateModelOrService(java.lang.Class<T>) can be used. But it cannot be used, because it needs an empty constructor... Much more work at LatexCitationsTabViewModel required.

Background: The FileUpdateMonitor should not be retrieved by globals, but via the injector. One would need in org.jabref.gui.entryeditor.LatexCitationsTab#LatexCitationsTab at the first line (where the ViewModel is instanitated) to use com.airhacks.afterburner.injection.Injector#instantiateModelOrService(java.lang.Class<T>).

[koppor]

Add the method also to the interface so that there is no need to cast in LatexCitationsViewModel

[koppor]

This is wrong. You really need to update the DefaultFileMonitorImplementation.

Here a test case that notifications for new files works:

diff --git a/src/main/java/org/jabref/gui/JabRefExecutorService.java b/src/main/java/org/jabref/gui/JabRefExecutorService.java
index 90c3b31bda..74eb0bbea5 100644
--- a/src/main/java/org/jabref/gui/JabRefExecutorService.java
+++ b/src/main/java/org/jabref/gui/JabRefExecutorService.java
@@ -184,12 +184,12 @@ public class JabRefExecutorService {
         try {
             // This is non-blocking. See https://stackoverflow.com/a/57383461/873282.
             executorService.shutdown();
-            if (!executorService.awaitTermination(60, TimeUnit.SECONDS)) {
-                LOGGER.debug("One minute passed, {} still not completed. Trying forced shutdown.", executorService.toString());
+            if (!executorService.awaitTermination(10, TimeUnit.SECONDS)) {
+                LOGGER.debug("10 seconds, {} still not completed. Trying forced shutdown.", executorService.toString());
                 // those threads will be interrupted in their current task
                 executorService.shutdownNow();
-                if (executorService.awaitTermination(60, TimeUnit.SECONDS)) {
-                    LOGGER.debug("One minute passed again - forced shutdown of {} worked.", executorService.toString());
+                if (executorService.awaitTermination(10, TimeUnit.SECONDS)) {
+                    LOGGER.debug("10 seconds passed again - forced shutdown of {} worked.", executorService.toString());
                 } else {
                     LOGGER.error("{} did not terminate", executorService.toString());
                 }
diff --git a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java
index 0090539a96..bcb88189ef 100644
--- a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java
+++ b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java
@@ -8,6 +8,8 @@ import java.nio.file.StandardWatchEventKinds;
 import java.nio.file.WatchEvent;
 import java.nio.file.WatchKey;
 import java.nio.file.WatchService;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -32,8 +34,8 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(DefaultFileUpdateMonitor.class);
 
-    private final Multimap<Path, FileUpdateListener> listeners = ArrayListMultimap.create(20, 4);
-    private volatile WatchService watcher;
+    volatile WatchService watcher;
+    final Multimap<Path, FileUpdateListener> listeners = ArrayListMultimap.create(20, 4);
     private final AtomicBoolean notShutdown = new AtomicBoolean(true);
     private final AtomicReference<Optional<JabRefException>> filesystemMonitorFailure = new AtomicReference<>(Optional.empty());
 
@@ -82,7 +84,12 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor {
     }
 
     private void notifyAboutChange(Path path) {
-        listeners.get(path).forEach(FileUpdateListener::fileUpdated);
+        Collection<FileUpdateListener> fileUpdateListeners = Collections.emptyList();
+        while ((path != null) && (fileUpdateListeners.isEmpty())) {
+            fileUpdateListeners = listeners.get(path);
+            path = path.getParent();
+        }
+        fileUpdateListeners.forEach(FileUpdateListener::fileUpdated);
     }
 
     @Override
@@ -126,6 +133,7 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor {
             if (watcher != null) {
                 watcher.close();
             }
+            Thread.currentThread().interrupt();
         } catch (IOException e) {
             LOGGER.error("error closing watcher", e);
         }
diff --git a/src/test/java/org/jabref/gui/util/DefaultFileUpdateMonitorTest.java b/src/test/java/org/jabref/gui/util/DefaultFileUpdateMonitorTest.java
new file mode 100644
index 0000000000..95c6235738
--- /dev/null
+++ b/src/test/java/org/jabref/gui/util/DefaultFileUpdateMonitorTest.java
@@ -0,0 +1,40 @@
+package org.jabref.gui.util;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.jabref.gui.JabRefExecutorService;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class DefaultFileUpdateMonitorTest {
+
+    @Test
+    void demonstrateListenerForDirectory(@TempDir Path rootPath) throws Exception {
+        DefaultFileUpdateMonitor fileUpdateMonitor = new DefaultFileUpdateMonitor();
+        JabRefExecutorService.INSTANCE.executeInterruptableTask(fileUpdateMonitor, "FileUpdateMonitor");
+        while (fileUpdateMonitor.watcher == null) {
+            Thread.sleep(100);
+            Thread.yield();
+        }
+        rootPath.register(fileUpdateMonitor.watcher, StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_MODIFY);
+        final AtomicBoolean called = new AtomicBoolean(false);
+        fileUpdateMonitor.listeners.put(rootPath, () -> {
+            called.set(true);
+        });
+        Files.createFile(rootPath.resolve("test.txt"));
+        int callCount = 0;
+        while ((callCount < 10) && (!called.get())) {
+            Thread.sleep(100);
+            Thread.yield();
+            callCount++;
+        }
+        JabRefExecutorService.INSTANCE.shutdownEverything();
+        assertTrue(called.get());
+    }
+}

Maybe, you should add the Path of the changed file to FileUpdateListener.

Other approach: implement other classes handling directories and ensure that DefaultFileUpdateMonitor handles files only.

[koppor]

@rachedkko Thank you for working on this. Please do not reference the issue/PR in a commit. This is automatically done when pushing to the branch. - Moreover, please check the tests output (or the "Files changed" tab in this PR. You will see some (minor) checkstyle errors. You can also have checkstyle running within IntelliJ.

[koppor]

I spent work to directly provide Java code updates. Please commit them (if you agree - if you don't, please tell why)

Example:

(You can see that at https://github.com/JabRef/jabref/pull/10937/files)

[koppor]

This import can be removed, can't it?

[koppor]

TBH, currently this is 20% of the work. The real work is following:

• Refactor org.jabref.gui.entryeditor.LatexCitationsTabViewModel: Move away org.jabref.gui.entryeditor.LatexCitationsTabViewModel#searchDirectory, org.jabref.gui.entryeditor.LatexCitationsTabViewModel#searchAndParse as class CitationFinder in package org.jabref.logic.texparser. -- Also test cases need to be added
• Refactor org.jabref.model.texparser.LatexParserResult to allow partial updates. That means: If file X is changed, call new DefaultLatexParser().parse(X) and change the contents of org.jabref.model.texparser.LatexParserResult#citations accordingly. IMHO, a map from Path to Set<Citations> (Multimap) should be kept. The value before the update and after the update be stored. A diff calculated. That diff should be applied to citations. - The method update can be implemented in LatexParserResult. - The class LatexParserResult should be renamed in CitationState.
• A new file watcher, which works on directories and handles both file editing and creation of new files. On both events, CitationState#update(path) can be called. That method should simply add the file to the currently known files. -- Deletion has to be handled separately. event: delete, new method CitationState#remove(path) called

Reasons: Energy saving. Very little CPU usage for an update on one file. Although your current method works, it parses ALL .tex files again, and handles the content. Large LaTeX projects have dozens of tex files, which are very long. This costs time.

Please either work on my proposals above - or "just" fix the compile errors above to have an initial version OKish for an initial student contribution. I think, we won't merge that - and hope for someone to do the hard work... (With the above guidelines it should not be that hard though)

[VinterSallad]

Hi, we won't be able to see this issue through sadly, it turned out to be something beyond the time frame given to us for the assignment in the course.

But we really appreciate the learning opportunity you all gave us, it was really eye-opening.

[koppor]

@VinterSallad Thank you for your honest repsonse. We will close the PR and update the issue accordingly.