-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add FileMonitor for LaTeX citations #10937
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
Changes from all commits
426b0bf
c2370b8
4eaf252
b4fa3a9
3e0f7a5
3e7fb9e
14880ac
6c1bb03
ce93345
71ea781
59797e5
ba8adec
fdd6750
dc161ca
ad19f5a
8ff512e
afb4c09
7f3b5f2
653660e
6f2982d
2fd55bb
2ea59b5
07423e8
f3d3b90
51848ad
7d1aa43
0eff465
c3e9b13
ac4950e
9318193
271e62c
632f250
d0c1e0c
794291c
79027ee
968bcf6
d670888
c0b171e
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 |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ | |
| 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; | ||
| final Multimap<Path, FileUpdateListener> listeners = ArrayListMultimap.create(20, 4); | ||
| volatile WatchService watcher; | ||
| private final AtomicBoolean notShutdown = new AtomicBoolean(true); | ||
| private final AtomicReference<Optional<JabRefException>> filesystemMonitorFailure = new AtomicReference<>(Optional.empty()); | ||
|
|
||
|
|
@@ -82,7 +84,12 @@ public boolean isActive() { | |
| } | ||
|
|
||
| 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 | ||
|
|
@@ -97,6 +104,22 @@ public void addListenerForFile(Path file, FileUpdateListener listener) throws IO | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Add a new directory to monitor. | ||
| * | ||
| * @param directory The directory to monitor. | ||
| * @throws IOException if the directory does not exist. | ||
| */ | ||
| public void addListenerForDirectory(Path directory, FileUpdateListener listener) throws IOException { | ||
|
Member
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. Add the method also to the interface so that there is no need to cast in LatexCitationsViewModel |
||
| // This function was created because it makes more sense to call addListenerForDirectory when | ||
| // the Path is a directory and not a file, even though it does the same thing as addListenerForFile. | ||
| // The function addListenerForFile works for directories as well. We have to listen to the parent | ||
| // directory in this case too otherwise if a new file is created in the child directory, no | ||
|
Member
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. 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 Other approach: implement other classes handling directories and ensure that |
||
| // event will be triggered as the path linked to the event doesn't match any paths in the MultiMap. | ||
|
|
||
| addListenerForFile(directory, listener); | ||
| } | ||
|
|
||
| @Override | ||
| public void removeListener(Path path, FileUpdateListener listener) { | ||
| listeners.remove(path, listener); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package org.jabref.logic.texparser; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Collection; | ||
| import java.util.List; | ||
|
|
||
| import javafx.beans.property.ObjectProperty; | ||
|
|
||
| import org.jabref.gui.entryeditor.LatexCitationsTabViewModel; | ||
| import org.jabref.logic.util.io.FileUtil; | ||
| import org.jabref.model.database.BibDatabaseContext; | ||
| import org.jabref.model.texparser.Citation; | ||
| import org.jabref.model.texparser.LatexParserResult; | ||
| import org.jabref.preferences.PreferencesService; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class CitationFinder { | ||
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(LatexCitationsTabViewModel.class); | ||
| private static final String TEX_EXT = ".tex"; | ||
| private LatexParserResult latexParserResult; | ||
|
|
||
| public Collection<Citation> searchAndParse(BibDatabaseContext databaseContext, PreferencesService preferencesService, ObjectProperty<Path> directory, String citeKey) throws IOException { | ||
| // we need to check whether the user meanwhile set the LaTeX file directory or the database changed locations | ||
| Path newDirectory = databaseContext.getMetaData().getLatexFileDirectory(preferencesService.getFilePreferences().getUserAndHost()) | ||
| .orElse(FileUtil.getInitialDirectory(databaseContext, preferencesService.getFilePreferences().getWorkingDirectory())); | ||
|
|
||
| if (latexParserResult == null || !newDirectory.equals(directory.get())) { | ||
| directory.set(newDirectory); | ||
|
|
||
| if (!newDirectory.toFile().exists()) { | ||
| throw new IOException("Current search directory does not exist: %s".formatted(newDirectory)); | ||
| } | ||
|
|
||
| List<Path> texFiles = searchDirectory(newDirectory); | ||
| LOGGER.debug("Found tex files: {}", texFiles); | ||
| latexParserResult = new DefaultLatexParser().parse(texFiles); | ||
| } | ||
|
|
||
| return latexParserResult.getCitationsByKey(citeKey); | ||
| } | ||
|
|
||
| /** | ||
| * @param directory the directory to search for. It is recursively searched. | ||
| */ | ||
| private List<Path> searchDirectory(Path directory) { | ||
| LOGGER.debug("Searching directory {}", directory); | ||
| try { | ||
| return Files.walk(directory) | ||
| .filter(Files::isRegularFile) | ||
| .filter(path -> path.toString().endsWith(TEX_EXT)) | ||
| .toList(); | ||
| } catch (IOException e) { | ||
| LOGGER.error("Error while searching files", e); | ||
| return List.of(); | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.