Skip to content
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

Add directory monitor for Latex citations #11245

Merged
merged 11 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We added tooltip on main table cells that shows cell content or cell content and entry preview if set in preferences. [10925](https://github.com/JabRef/jabref/issues/10925)
- We added the ability to add a keyword/crossref when typing the separator character (e.g., comma) in the keywords/crossref fields. [#11178](https://github.com/JabRef/jabref/issues/11178)
- We added an exporter and improved the importer for Endnote XML format. [#11137](https://github.com/JabRef/jabref/issues/11137)
- We added support for automatically update LaTeX citations when a LaTeX file is created, removed, or modified. [#10585](https://github.com/JabRef/jabref/issues/10585)

### Changed

Expand Down
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ dependencies {
// YAML formatting
implementation 'org.yaml:snakeyaml:2.2'

implementation 'commons-io:commons-io:2.16.1'

testImplementation 'io.github.classgraph:classgraph:4.8.170'
testImplementation 'org.junit.jupiter:junit-jupiter:5.10.2'
testImplementation 'org.junit.platform:junit-platform-launcher:1.10.2'
Expand Down
2 changes: 0 additions & 2 deletions docs/decisions/0028-http-return-bibtex-string.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
nav_order: 28
parent: Decision Records
---
<!-- we need to disable MD025, because we use the different heading "ADR Template" in the homepage (see above) than it is foreseen in the template -->
<!-- markdownlint-disable-next-line MD025 -->
# Return BibTeX string and CSL Item JSON in the API

## Context and Problem Statement
Expand Down
5 changes: 1 addition & 4 deletions docs/decisions/0029-cff-export-multiple-entries.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
---
nav_order: 28
nav_order: 29
parent: Decision Records
---

<!-- we need to disable MD025, because we use the different heading "ADR Template" in the homepage (see above) than it is foreseen in the template -->
<!-- markdownlint-disable-next-line MD025 -->
# Exporting multiple entries to CFF

## Context and Problem Statement
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
nav_order: 30
parent: Decision Records
---
# Use Apache Commons IO for directory monitoring

## Context and Problem Statement

In JabRef, there is a need to add a directory monitor that will listen for changes in a specified directory.

Currently, the monitor is used to automatically update the [LaTeX Citations](https://docs.jabref.org/advanced/entryeditor/latex-citations) when a LaTeX file in the LaTeX directory is created, removed, or modified ([#10585](https://github.com/JabRef/jabref/issues/10585)).
Additionally, this monitor will be used to create a dynamic group that mirrors the file system structure ([#10930](https://github.com/JabRef/jabref/issues/10930)).

## Considered Options

* Use [java.nio.file.WatchService](https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/nio/file/WatchService.html)
* Use [io.methvin.watcher.DirectoryWatcher](https://github.com/gmethvin/directory-watcher)
* Use [org.apache.commons.io.monitor](https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/monitor/package-summary.html)

## Decision Outcome

Chosen option: "Use [org.apache.commons.io.monitor](https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/monitor/package-summary.html)", because comes out best (see below).

## Pros and Cons of the Options

### java.nio.file.WatchService

* Good, because it is a standard Java API for watching directories.
* Good, because it does not need polling, it is event-based for most operating systems.
* Bad, because:
1. Does not detect files coming together with a new folder (JDK issue: [JDK-8162948](https://bugs.openjdk.org/browse/JDK-8162948)).
2. Deleting a subdirectory does not detect deleted files in that directory.
3. Access denied when trying to delete the recursively watched directory on Windows (JDK issue: [JDK-6972833](https://bugs.openjdk.org/browse/JDK-6972833)).
4. Implemented on macOS by the generic `PollingWatchService`. (JDK issue: [JDK-8293067](https://bugs.openjdk.org/browse/JDK-8293067))

### io.methvin.watcher.DirectoryWatcher

* Good, because it implemented on top of the `java.nio.file.WatchService`, which is a standard Java API for watching directories.
* Good, because it resolves some of the issues of the `java.nio.file.WatchService`.
* Uses`ExtendedWatchEventModifier.FILE_TREE` on Windows, which resolves issues (1, 3) of the `java.nio.file.WatchService`.
* On macOS have native implementation based on the Carbon File System Events API, this resolves issue (4) of the `java.nio.file.WatchService`.
* Bad, because issue (2) of the `java.nio.file.WatchService` is not resolved.

### org.apache.commons.io.monitor

* Good, because there are no observed issues.
* Good, because can handle huge amount of files without overflowing.
* Bad, because it uses a polling mechanism at fixed intervals, which can waste CPU cycles if no change occurs.
23 changes: 15 additions & 8 deletions external-libraries.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,12 @@ Project: Apache Commons Digester
URL: https://commons.apache.org/proper/commons-digester/
```

```yaml
Id: commons-io:commons-io
Project: Apache Commons IO
URL: https://commons.apache.org/proper/commons-io/
```

```yaml
Id: de.rototor.jeuclid:jeuclid-core
Project: JEuclid
Expand Down Expand Up @@ -537,7 +543,7 @@ Id: com.ibm.icu:*
Project: International Components for Unicode
URL: https://icu.unicode.org/
License: Unicode License (https://www.unicode.org/copyright.html)
Note: Our own fork https://github.com/JabRef/icu. Upstream PR: https://github.com/unicode-org/icu/pull/2127
Note: Our own fork https://github.com/JabRef/icu. [Upstream PR](https://github.com/unicode-org/icu/pull/2127)
Path: lib/icu4j.jar
SourcePath: lib/ic4j-src.jar
```
Expand Down Expand Up @@ -579,49 +585,49 @@ License: LGPL-2.1-or-later

```yaml
Id: org.openjfx:javafx-base
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-controls
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-fxml
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-graphics
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-media
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-swing
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```

```yaml
Id: org.openjfx:javafx-web
Project JavaFX
Project: JavaFX
URL: https://openjfx.io/
License: GPL-2.0 WITH Classpath-exception-2.0
```
Expand Down Expand Up @@ -765,6 +771,7 @@ commons-cli:commons-cli:1.6.0
commons-codec:commons-codec:1.16.0
commons-collections:commons-collections:3.2.2
commons-digester:commons-digester:2.1
commons-io:commons-io:2.16.1
commons-logging:commons-logging:1.2
commons-validator:commons-validator:1.7
de.rototor.jeuclid:jeuclid-core:3.1.11
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
requires java.prefs;
requires com.fasterxml.aalto;

// YAML
requires org.yaml.snakeyaml;

// Annotations (@PostConstruct)
requires jakarta.annotation;
requires jakarta.inject;
Expand Down Expand Up @@ -88,13 +91,14 @@
uses org.mariadb.jdbc.credential.CredentialPlugin;

// Apache Commons and other (similar) helper libraries
requires com.google.common;
requires io.github.javadiffutils;
requires java.string.similarity;
requires org.apache.commons.cli;
requires org.apache.commons.csv;
requires org.apache.commons.io;
requires org.apache.commons.lang3;
requires org.apache.commons.text;
requires com.google.common;
requires io.github.javadiffutils;
requires java.string.similarity;

requires com.github.tomtung.latex2unicode;
requires fastparse;
Expand Down Expand Up @@ -146,5 +150,4 @@
requires de.saxsys.mvvmfx.validation;
requires dd.plist;
requires mslinks;
requires org.yaml.snakeyaml;
}
5 changes: 5 additions & 0 deletions src/main/java/org/jabref/gui/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ public static void shutdownThreadPools() {
if (fileUpdateMonitor != null) {
fileUpdateMonitor.shutdown();
}

if (stateManager.getDirectoryMonitor() != null) {
stateManager.getDirectoryMonitor().shutdown();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calixtus Can you check if the file DirectoryMonitor should reside in StateManager or if we should move it to Globals (similar to fileUpdateMonitor).

Copy link
Member

@calixtus calixtus Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither.

Globals is deprecated and is annoted as such. It shall not be used at all. We are working towards dissolving that class.

StateManager is not the new Globals.
The directory manager should be injected by constructor or in a view by the viewloader.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the File update monitor and the DirectoryMonitor be merged to a more generic class?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Globals points to StateManager:

/**
 * @deprecated try to use {@link StateManager} and {@link org.jabref.preferences.PreferencesService}
 */

We need a "Global" DirectoryMonitor. Would you place it in org.jabref.gui.DefaultInjector and then add a shutdown method to it?

We need to have a full lifecycle of DirectoryMonitor. Creation, running, and shutdown. There currently is no migration guide at org.jabref.gui.Globals#shutdownThreadPools

To keep things going, I vote to move the currently directoryMonitor code into Globals and do a refactoring towards dependency injection afterwards.


Also the more generic ones migration away from FileUpdateMonitor to Apache Commons IO or other classes should be done later. Otherwise, IMHO the PR blows up only because of "clean code".


Side comment: Injector.registerExistingAndInject(this); is a neat thing. I wonder why we don't use that more often :p.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets go for Globals and then refactor it afterwards, so this PR keeps its focus,


JabRefExecutorService.INSTANCE.shutdownEverything();
}

Expand Down
16 changes: 14 additions & 2 deletions src/main/java/org/jabref/gui/LibraryTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.util.DirectoryMonitorManager;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.PreferencesService;

Expand Down Expand Up @@ -127,9 +128,9 @@ private enum PanelMode { MAIN_TABLE, MAIN_TABLE_AND_ENTRY_EDITOR }

// Indicates whether the tab is loading data using a dataloading task
// The constructors take care to the right true/false assignment during start.
private SimpleBooleanProperty loading = new SimpleBooleanProperty(false);
private final SimpleBooleanProperty loading = new SimpleBooleanProperty(false);

// initally, the dialog is loading, not saving
// initially, the dialog is loading, not saving
private boolean saving = false;

private PersonNameSuggestionProvider searchAutoCompleter;
Expand All @@ -151,6 +152,7 @@ private enum PanelMode { MAIN_TABLE, MAIN_TABLE_AND_ENTRY_EDITOR }

private final IndexingTaskManager indexingTaskManager;
private final TaskExecutor taskExecutor;
private final DirectoryMonitorManager directoryMonitorManager;

private LibraryTab(BibDatabaseContext bibDatabaseContext,
LibraryTabContainer tabContainer,
Expand All @@ -171,6 +173,7 @@ private LibraryTab(BibDatabaseContext bibDatabaseContext,
this.entryTypesManager = entryTypesManager;
this.indexingTaskManager = new IndexingTaskManager(taskExecutor);
this.taskExecutor = taskExecutor;
this.directoryMonitorManager = new DirectoryMonitorManager(stateManager.getDirectoryMonitor());

bibDatabaseContext.getDatabase().registerListener(this);
bibDatabaseContext.getMetaData().registerListener(this);
Expand Down Expand Up @@ -832,6 +835,11 @@ private void onClosed(Event event) {
} catch (RuntimeException e) {
LOGGER.error("Problem when closing change monitor", e);
}
try {
directoryMonitorManager.unregister();
} catch (RuntimeException e) {
LOGGER.error("Problem when closing directory monitor", e);
}
try {
PdfIndexerManager.shutdownIndexer(bibDatabaseContext);
} catch (RuntimeException e) {
Expand Down Expand Up @@ -864,6 +872,10 @@ public BibDatabaseContext getBibDatabaseContext() {
return this.bibDatabaseContext;
}

public DirectoryMonitorManager getDirectoryMonitorManager() {
return directoryMonitorManager;
}

public boolean isSaving() {
return saving;
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/jabref/gui/StateManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.jabref.gui.sidepane.SidePaneType;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.CustomLocalDragboard;
import org.jabref.gui.util.DefaultDirectoryMonitor;
import org.jabref.gui.util.DialogWindowState;
import org.jabref.gui.util.OptionalObjectProperty;
import org.jabref.logic.search.SearchQuery;
Expand Down Expand Up @@ -73,6 +74,7 @@ public class StateManager {
private final ObjectProperty<LastAutomaticFieldEditorEdit> lastAutomaticFieldEditorEdit = new SimpleObjectProperty<>();

private final ObservableList<String> searchHistory = FXCollections.observableArrayList();
private final DefaultDirectoryMonitor directoryMonitor = new DefaultDirectoryMonitor();

public StateManager() {
activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElseOpt(null)));
Expand Down Expand Up @@ -190,7 +192,7 @@ public ObservableList<Task<?>> getBackgroundTasks() {
}

public void addBackgroundTask(BackgroundTask<?> backgroundTask, Task<?> task) {
this.backgroundTasks.add(0, new Pair<>(backgroundTask, task));
this.backgroundTasks.addFirst(new Pair<>(backgroundTask, task));
}

public EasyBinding<Boolean> getAnyTaskRunning() {
Expand Down Expand Up @@ -255,4 +257,8 @@ public List<String> getLastSearchHistory(int size) {
public void clearSearchHistory() {
searchHistory.clear();
}

public DefaultDirectoryMonitor getDirectoryMonitor() {
return directoryMonitor;
}
}
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.Field;
import org.jabref.model.util.DirectoryMonitorManager;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.PreferencesService;

Expand Down Expand Up @@ -82,6 +83,7 @@ public class EntryEditor extends BorderPane {
private final BibDatabaseContext databaseContext;
private final EntryEditorPreferences entryEditorPreferences;
private final ExternalFilesEntryLinker fileLinker;
private final DirectoryMonitorManager directoryMonitorManager;

private Subscription typeSubscription;

Expand Down Expand Up @@ -121,6 +123,7 @@ public class EntryEditor extends BorderPane {
public EntryEditor(LibraryTab libraryTab) {
this.libraryTab = libraryTab;
this.databaseContext = libraryTab.getBibDatabaseContext();
this.directoryMonitorManager = libraryTab.getDirectoryMonitorManager();

ViewLoader.view(this)
.root(this)
Expand Down Expand Up @@ -307,7 +310,7 @@ private List<EntryEditorTab> createTabs() {
bibEntryTypesManager,
keyBindingRepository);
entryEditorTabs.add(sourceTab);
entryEditorTabs.add(new LatexCitationsTab(databaseContext, preferencesService, taskExecutor, dialogService));
entryEditorTabs.add(new LatexCitationsTab(databaseContext, preferencesService, dialogService, directoryMonitorManager));
entryEditorTabs.add(new FulltextSearchResultsTab(stateManager, preferencesService, dialogService, taskExecutor));

return entryEditorTabs;
Expand Down
Loading
Loading