Fix ArgumentProcessor (JabRef#11728)

* Refine JavaDoc * Add TODOs Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> * Fix typos * Reoder settings * Fix arch flaw Co-authored-by: Christoph <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> * Fix checkstyle and formatting --------- Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: Christoph <siedlerkiller@gmail.com>
leaf-soba · Sep 8, 2024 · 8b168ee · 8b168ee
1 parent 51f04a3
commit 8b168ee
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 47 deletions.
diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java
@@ -14,7 +14,6 @@
 import java.util.prefs.BackingStoreException;
 
 import org.jabref.gui.externalfiles.AutoSetFileLinksUtil;
-import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.util.CurrentThreadTaskExecutor;
 import org.jabref.logic.JabRefException;
 import org.jabref.logic.UiCommand;
@@ -99,16 +98,13 @@ public ArgumentProcessor(String[] args,
     }
 
     /**
-     * Will open a file (like importFile), but will also request JabRef to focus on this database
+     * Will open a file (like {@link #importFile(String)}, but will also request JabRef to focus on this database.
      *
-     * @param argument See importFile.
      * @return ParserResult with setToOpenTab(true)
      */
-    private Optional<ParserResult> importToOpenBase(String argument) {
-        Optional<ParserResult> result = importFile(argument);
-
+    private Optional<ParserResult> importToOpenBase(String importArguments) {
+        Optional<ParserResult> result = importFile(importArguments);
         result.ifPresent(ParserResult::setToOpenTab);
-
         return result;
     }
 
@@ -125,9 +121,13 @@ private Optional<ParserResult> importBibtexToOpenBase(String argument, ImportFor
         }
     }
 
-    private Optional<ParserResult> importFile(String argument) {
-        LOGGER.debug("Importing file {}", argument);
-        String[] data = argument.split(",");
+    /**
+     *
+     * @param importArguments Format: <code>fileName[,format]</code>
+     */
+    private Optional<ParserResult> importFile(String importArguments) {
+        LOGGER.debug("Importing file {}", importArguments);
+        String[] data = importArguments.split(",");
 
         String address = data[0];
         Path file;
@@ -295,7 +295,7 @@ public void processArguments() {
     }
 
     private void writeMetadataToPdf(List<ParserResult> loaded,
-                                    String filesAndCitekeys,
+                                    String filesAndCiteKeys,
                                     XmpPreferences xmpPreferences,
                                     FilePreferences filePreferences,
                                     BibDatabaseMode databaseMode,
@@ -314,7 +314,7 @@ private void writeMetadataToPdf(List<ParserResult> loaded,
         XmpPdfExporter xmpPdfExporter = new XmpPdfExporter(xmpPreferences);
         EmbeddedBibFilePdfExporter embeddedBibFilePdfExporter = new EmbeddedBibFilePdfExporter(databaseMode, entryTypesManager, fieldPreferences);
 
-        if ("all".equals(filesAndCitekeys)) {
+        if ("all".equals(filesAndCiteKeys)) {
             for (BibEntry entry : databaseContext.getEntries()) {
                 writeMetadataToPDFsOfEntry(
                         databaseContext,
@@ -332,7 +332,7 @@ private void writeMetadataToPdf(List<ParserResult> loaded,
 
         List<String> citeKeys = new ArrayList<>();
         List<String> pdfs = new ArrayList<>();
-        for (String fileOrCiteKey : filesAndCitekeys.split(",")) {
+        for (String fileOrCiteKey : filesAndCiteKeys.split(",")) {
             if (fileOrCiteKey.toLowerCase(Locale.ROOT).endsWith(".pdf")) {
                 pdfs.add(fileOrCiteKey);
             } else {
@@ -368,7 +368,7 @@ private void writeMetadataToPDFsOfEntry(BibDatabaseContext databaseContext,
                                             EmbeddedBibFilePdfExporter embeddedBibFilePdfExporter,
                                             JournalAbbreviationRepository abbreviationRepository,
                                             boolean writeXMP,
-                                            boolean embeddBibfile) {
+                                            boolean embedBibfile) {
         try {
             if (writeXMP) {
                 if (xmpPdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
@@ -377,7 +377,7 @@ private void writeMetadataToPDFsOfEntry(BibDatabaseContext databaseContext,
                     System.err.printf("Cannot write XMP metadata on any linked files of %s. Make sure there is at least one linked file and the path is correct.%n", citeKey);
                 }
             }
-            if (embeddBibfile) {
+            if (embedBibfile) {
                 if (embeddedBibFilePdfExporter.exportToAllFilesOfEntry(databaseContext, filePreferences, entry, List.of(entry), abbreviationRepository)) {
                     System.out.printf("Successfully embedded metadata on at least one linked file of %s%n", citeKey);
                 } else {
@@ -461,6 +461,7 @@ private boolean exportMatches(List<ParserResult> loaded) {
 
         List<BibEntry> matches;
         try {
+            // extract current thread task executor from luceneManager
             matches = new DatabaseSearcher(query, databaseContext, new CurrentThreadTaskExecutor(), preferencesService.getFilePreferences()).getMatches();
         } catch (IOException e) {
             LOGGER.error("Error occurred when searching", e);
@@ -729,12 +730,17 @@ private void resetPreferences(String value) {
     private void automaticallySetFileLinks(List<ParserResult> loaded) {
         for (ParserResult parserResult : loaded) {
             BibDatabase database = parserResult.getDatabase();
-            LOGGER.info(Localization.lang("Automatically setting file links"));
+            LOGGER.info("Automatically setting file links for {}",
+                    parserResult.getDatabaseContext().getDatabasePath()
+                                .map(Path::getFileName)
+                                .map(Path::toString).orElse("UNKNOWN"));
+
             AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(
                     parserResult.getDatabaseContext(),
                     preferencesService.getFilePreferences(),
                     preferencesService.getAutoLinkPreferences());
-            util.linkAssociatedFiles(database.getEntries(), new NamedCompound(""));
+
+            util.linkAssociatedFiles(database.getEntries(), (linkedFile, bibEntry) -> bibEntry.addFile(linkedFile));
         }
     }
 

diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoLinkFilesAction.java b/src/main/java/org/jabref/gui/externalfiles/AutoLinkFilesAction.java
@@ -1,6 +1,7 @@
 package org.jabref.gui.externalfiles;
 
 import java.util.List;
+import java.util.function.BiConsumer;
 
 import javax.swing.undo.UndoManager;
 
@@ -10,11 +11,15 @@
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.undo.NamedCompound;
+import org.jabref.gui.undo.UndoableFieldChange;
 import org.jabref.gui.util.BindingsHelper;
 import org.jabref.gui.util.TaskExecutor;
+import org.jabref.logic.bibtex.FileFieldWriter;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.LinkedFile;
+import org.jabref.model.entry.field.StandardField;
 import org.jabref.preferences.PreferencesService;
 
 import static org.jabref.gui.actions.ActionHelper.needsDatabase;
@@ -47,16 +52,26 @@ public AutoLinkFilesAction(DialogService dialogService, PreferencesService prefe
     public void execute() {
         final BibDatabaseContext database = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("Database null"));
         final List<BibEntry> entries = stateManager.getSelectedEntries();
-        final AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(
+
+        AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(
                 database,
                 preferences.getFilePreferences(),
                 preferences.getAutoLinkPreferences());
         final NamedCompound nc = new NamedCompound(Localization.lang("Automatically set file links"));
 
         Task<AutoSetFileLinksUtil.LinkFilesResult> linkFilesTask = new Task<>() {
+            final BiConsumer<LinkedFile, BibEntry> onLinkedFile = (linkedFile, entry) -> {
+                // lambda for gui actions that are relevant when setting the linked file entry when ui is opened
+                String newVal = FileFieldWriter.getStringRepresentation(linkedFile);
+                String oldVal = entry.getField(StandardField.FILE).orElse(null);
+                UndoableFieldChange fieldChange = new UndoableFieldChange(entry, StandardField.FILE, oldVal, newVal);
+                nc.addEdit(fieldChange); // push to undo manager is in succeeded
+                entry.addFile(linkedFile);
+            };
+
             @Override
             protected AutoSetFileLinksUtil.LinkFilesResult call() {
-                return util.linkAssociatedFiles(entries, nc);
+                return util.linkAssociatedFiles(entries, onLinkedFile);
             }
 
             @Override

diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java
@@ -6,22 +6,18 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.BiConsumer;
 
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.externalfiletype.UnknownExternalFileType;
-import org.jabref.gui.undo.NamedCompound;
-import org.jabref.gui.undo.UndoableFieldChange;
-import org.jabref.gui.util.UiTaskExecutor;
-import org.jabref.logic.bibtex.FileFieldWriter;
 import org.jabref.logic.util.io.AutoLinkPreferences;
 import org.jabref.logic.util.io.FileFinder;
 import org.jabref.logic.util.io.FileFinders;
 import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.LinkedFile;
-import org.jabref.model.entry.field.StandardField;
 import org.jabref.preferences.FilePreferences;
 
 import org.slf4j.Logger;
@@ -66,7 +62,7 @@ private AutoSetFileLinksUtil(List<Path> directories, FilePreferences filePrefere
         this.filePreferences = filePreferences;
     }
 
-    public LinkFilesResult linkAssociatedFiles(List<BibEntry> entries, NamedCompound ce) {
+    public LinkFilesResult linkAssociatedFiles(List<BibEntry> entries, BiConsumer<LinkedFile, BibEntry> onAddLinkedFile) {
         LinkFilesResult result = new LinkFilesResult();
 
         for (BibEntry entry : entries) {
@@ -79,26 +75,12 @@ public LinkFilesResult linkAssociatedFiles(List<BibEntry> entries, NamedCompound
                 LOGGER.error("Problem finding files", e);
             }
 
-            if (ce != null) {
-                boolean changed = false;
-
-                for (LinkedFile linkedFile : linkedFiles) {
-                    // store undo information
-                    String newVal = FileFieldWriter.getStringRepresentation(linkedFile);
-                    String oldVal = entry.getField(StandardField.FILE).orElse(null);
-                    UndoableFieldChange fieldChange = new UndoableFieldChange(entry, StandardField.FILE, oldVal, newVal);
-                    ce.addEdit(fieldChange);
-                    changed = true;
-
-                    UiTaskExecutor.runInJavaFXThread(() -> {
-                        entry.addFile(linkedFile);
-                    });
-                }
-
-                if (changed) {
-                    result.addBibEntry(entry);
-                }
+            for (LinkedFile linkedFile : linkedFiles) {
+                // store undo information
+                onAddLinkedFile.accept(linkedFile, entry);
             }
+
+            result.addBibEntry(entry);
         }
         return result;
     }

diff --git a/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java b/src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java
@@ -21,6 +21,7 @@
 import org.jabref.gui.LibraryTab;
 import org.jabref.gui.LibraryTabContainer;
 import org.jabref.gui.StateManager;
+import org.jabref.gui.externalfiles.AutoLinkFilesAction;
 import org.jabref.gui.importer.ImportEntriesDialog;
 import org.jabref.gui.importer.ParserResultWarningDialog;
 import org.jabref.gui.importer.actions.OpenDatabaseAction;
@@ -158,7 +159,14 @@ public void handleUiCommands(List<UiCommand> uiCommands) {
                     .forEach(command -> openDatabases(command.parserResults()));
         }
 
+        // Handle automatically setting file links
+        uiCommands.stream()
+                  .filter(UiCommand.AutoSetFileLinks.class::isInstance).findAny()
+                  .map(UiCommand.AutoSetFileLinks.class::cast)
+                  .ifPresent(autoSetFileLinks -> autoSetFileLinks(autoSetFileLinks.parserResults()));
+
         // Handle jumpToEntry
+        // Needs to go last, because it requires all libraries opened
         uiCommands.stream()
                   .filter(UiCommand.JumpToEntryKey.class::isInstance)
                   .map(UiCommand.JumpToEntryKey.class::cast)
@@ -172,14 +180,13 @@ public void handleUiCommands(List<UiCommand> uiCommands) {
     }
 
     private void openDatabases(List<ParserResult> parserResults) {
-        final List<ParserResult> failed = new ArrayList<>();
         final List<ParserResult> toOpenTab = new ArrayList<>();
 
         // Remove invalid databases
         List<ParserResult> invalidDatabases = parserResults.stream()
                                                            .filter(ParserResult::isInvalid)
                                                            .toList();
-        failed.addAll(invalidDatabases);
+        final List<ParserResult> failed = new ArrayList<>(invalidDatabases);
         parserResults.removeAll(invalidDatabases);
 
         // passed file (we take the first one) should be focused
@@ -397,4 +404,10 @@ void addImportedEntries(final LibraryTab tab, final ParserResult parserResult) {
         dialog.setTitle(Localization.lang("Import"));
         dialogService.showCustomDialogAndWait(dialog);
     }
+
+    void autoSetFileLinks(List<ParserResult> loaded) {
+        for (ParserResult parserResult : loaded) {
+            new AutoLinkFilesAction(dialogService, preferences, stateManager, undoManager, taskExecutor).execute();
+        }
+    }
 }
diff --git a/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java b/src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java
@@ -60,12 +60,12 @@ public class BibEntryTableViewModel {
 
     public BibEntryTableViewModel(BibEntry entry, BibDatabaseContext bibDatabaseContext, ObservableValue<MainTableFieldValueFormatter> fieldValueFormatter) {
         this.entry = entry;
+        this.bibDatabaseContext = bibDatabaseContext;
         this.fieldValueFormatter = fieldValueFormatter;
 
         this.linkedFiles = getField(StandardField.FILE).mapOpt(FileFieldParser::parse).orElseOpt(Collections.emptyList());
         this.linkedIdentifiers = createLinkedIdentifiersBinding(entry);
         this.matchedGroups = createMatchedGroupsBinding(bibDatabaseContext, entry);
-        this.bibDatabaseContext = bibDatabaseContext;
     }
 
     private static EasyBinding<Map<Field, String>> createLinkedIdentifiersBinding(BibEntry entry) {

diff --git a/src/main/java/org/jabref/logic/UiCommand.java b/src/main/java/org/jabref/logic/UiCommand.java
@@ -10,4 +10,6 @@ record BlankWorkspace() implements UiCommand { }
     record JumpToEntryKey(String citationKey) implements UiCommand { }
 
     record OpenDatabases(List<ParserResult> parserResults) implements UiCommand { }
+
+    record AutoSetFileLinks(List<ParserResult> parserResults) implements UiCommand { }
 }
diff --git a/src/main/java/org/jabref/logic/search/DatabaseSearcher.java b/src/main/java/org/jabref/logic/search/DatabaseSearcher.java
@@ -22,6 +22,7 @@ public class DatabaseSearcher {
     private final SearchQuery query;
     private final LuceneManager luceneManager;
 
+    // get rid of task executor here or add a constuctor overload?
     public DatabaseSearcher(SearchQuery query, BibDatabaseContext databaseContext, TaskExecutor taskExecutor, FilePreferences filePreferences) throws IOException {
         this.databaseContext = databaseContext;
         this.query = Objects.requireNonNull(query);