Fix: Refactor PushToApplications and split into logic and GUI

jabgui/src/main/java/org/jabref/gui/push/GuiPushToApplicationCommand.java

@@ -3,7 +3,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 
 import javafx.scene.control.ButtonBase;
 import javafx.scene.control.MenuItem;
@@ -19,7 +18,6 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.util.BackgroundTask;
 import org.jabref.logic.util.TaskExecutor;
-import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.strings.StringUtil;
 
@@ -95,26 +93,6 @@ public Action getAction() {
         return application.getAction();
     }
 
-    private static String getKeyString(List<BibEntry> entries, String delimiter) {
-        StringBuilder result = new StringBuilder();
-        Optional<String> citeKey;
-        boolean first = true;
-        for (BibEntry bes : entries) {
-            citeKey = bes.getCitationKey();
-            if (citeKey.isEmpty() || citeKey.get().isEmpty()) {
-                LOGGER.warn("Should never occur, because we made sure that all entries have keys");
-                continue;
-            }
-            if (first) {
-                result.append(citeKey.get());
-                first = false;
-            } else {
-                result.append(delimiter).append(citeKey.get());
-            }
-        }
-        return result.toString();
-    }
-
     @Override
     public void execute() {
         // If required, check that all entries have citation keys defined:
@@ -137,7 +115,6 @@ public void execute() {
     }
 
     private void pushEntries() {
-        BibDatabaseContext database = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("Database null"));
-        application.pushEntries(database, stateManager.getSelectedEntries(), getKeyString(stateManager.getSelectedEntries(), application.getDelimiter()));
+        application.pushEntries(stateManager.getSelectedEntries());
     }
 }

jabgui/src/main/java/org/jabref/gui/push/GuiPushToApplications.java

@@ -32,12 +32,13 @@ public static List<GuiPushToApplication> getAllGUIApplications(DialogService dia
                 new GuiPushToTexShop(dialogService, preferences),
                 new GuiPushToVScode(dialogService, preferences)));
 
-        return APPLICATIONS;
+        return Collections.unmodifiableList(APPLICATIONS);
     }
 
+    /// In case object "preferences" changes for itself, that update won't be propagated. Does not harm as the current callers do not change that reference itself.
     public static Optional<GuiPushToApplication> getGUIApplicationByName(String applicationName, DialogService dialogService, PushToApplicationPreferences preferences) {
         return getAllGUIApplications(dialogService, preferences).stream()
-                                                                .filter(application -> application.getDisplayName().equals(applicationName))
+                                                                .filter(application -> application.getDisplayName().equalsIgnoreCase(applicationName))
                                                                 .findAny();
     }
 }

jabgui/src/test/java/org/jabref/gui/push/PushToEmacsTest.java

@@ -1,5 +1,6 @@
 package org.jabref.gui.push;
 
+import java.util.List;
 import java.util.Map;
 
 import javafx.beans.property.SimpleMapProperty;
@@ -9,6 +10,7 @@
 import org.jabref.gui.DialogService;
 import org.jabref.logic.os.OS;
 import org.jabref.logic.push.PushToApplicationPreferences;
+import org.jabref.model.entry.BibEntry;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
@@ -42,6 +44,10 @@ void setup() {
 
     @Test
     void pushEntries() {
-        pushToEmacs.pushEntries(null, null, "key1,key2");
+        pushToEmacs.pushEntries(
+                List.of(
+                        new BibEntry().withCitationKey("key1"),
+                        new BibEntry().withCitationKey("key2"))
+        );
     }
 }

jabgui/src/test/java/org/jabref/gui/push/PushToTeXworksTest.java

@@ -1,5 +1,6 @@
 package org.jabref.gui.push;
 
+import java.util.List;
 import java.util.Map;
 
 import javafx.beans.property.SimpleMapProperty;
@@ -9,6 +10,7 @@
 import org.jabref.gui.DialogService;
 import org.jabref.logic.push.CitationCommandString;
 import org.jabref.logic.push.PushToApplicationPreferences;
+import org.jabref.model.entry.BibEntry;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -80,7 +82,7 @@ void pushEntries() {
         String testKey = "TestKey";
         String[] expectedCommand = new String[] {TEXWORKS_CLIENT_PATH, "--insert-text", testKey};
 
-        pushToTeXworks.pushEntries(null, null, testKey, processBuilder);
+        pushToTeXworks.pushEntries(List.of(new BibEntry().withCitationKey(testKey)), processBuilder);
 
         verify(processBuilder).command(expectedCommand);
     }

jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java

@@ -82,9 +82,9 @@
 import org.jabref.logic.os.OS;
 import org.jabref.logic.protectedterms.ProtectedTermsLoader;
 import org.jabref.logic.protectedterms.ProtectedTermsPreferences;
+import org.jabref.logic.push.Applications;
 import org.jabref.logic.push.CitationCommandString;
 import org.jabref.logic.push.PushToApplicationPreferences;
-import org.jabref.logic.push.PushToApplications;
 import org.jabref.logic.remote.RemotePreferences;
 import org.jabref.logic.search.SearchPreferences;
 import org.jabref.logic.shared.prefs.SharedDatabasePreferences;
@@ -793,15 +793,15 @@ public PushToApplicationPreferences getPushToApplicationPreferences() {
         Map<String, String> applicationCommands = new HashMap<>();
         // getEmptyIsDefault is used to ensure that an installation of a tool leads to the new path (instead of leaving the empty one)
         // Reason: empty string is returned by org.jabref.gui.desktop.os.Windows.detectProgramPath if program is not found. That path is stored in the preferences.
-        applicationCommands.put(PushToApplications.EMACS, getEmptyIsDefault(PUSH_EMACS_PATH));
-        applicationCommands.put(PushToApplications.LYX, getEmptyIsDefault(PUSH_LYXPIPE));
-        applicationCommands.put(PushToApplications.TEXMAKER, getEmptyIsDefault(PUSH_TEXMAKER_PATH));
-        applicationCommands.put(PushToApplications.TEXSTUDIO, getEmptyIsDefault(PUSH_TEXSTUDIO_PATH));
-        applicationCommands.put(PushToApplications.TEXWORKS, getEmptyIsDefault(PUSH_TEXWORKS_PATH));
-        applicationCommands.put(PushToApplications.VIM, getEmptyIsDefault(PUSH_VIM));
-        applicationCommands.put(PushToApplications.WIN_EDT, getEmptyIsDefault(PUSH_WINEDT_PATH));
-        applicationCommands.put(PushToApplications.SUBLIME_TEXT, getEmptyIsDefault(PUSH_SUBLIME_TEXT_PATH));
-        applicationCommands.put(PushToApplications.VSCODE, getEmptyIsDefault(PUSH_VSCODE_PATH));
+        applicationCommands.put(Applications.EMACS.getDisplayName(), getEmptyIsDefault(PUSH_EMACS_PATH));
+        applicationCommands.put(Applications.LYX.getDisplayName(), getEmptyIsDefault(PUSH_LYXPIPE));
+        applicationCommands.put(Applications.TEXMAKER.getDisplayName(), getEmptyIsDefault(PUSH_TEXMAKER_PATH));
+        applicationCommands.put(Applications.TEXSTUDIO.getDisplayName(), getEmptyIsDefault(PUSH_TEXSTUDIO_PATH));
+        applicationCommands.put(Applications.TEXWORKS.getDisplayName(), getEmptyIsDefault(PUSH_TEXWORKS_PATH));
+        applicationCommands.put(Applications.VIM.getDisplayName(), getEmptyIsDefault(PUSH_VIM));
+        applicationCommands.put(Applications.WIN_EDT.getDisplayName(), getEmptyIsDefault(PUSH_WINEDT_PATH));
+        applicationCommands.put(Applications.SUBLIME_TEXT.getDisplayName(), getEmptyIsDefault(PUSH_SUBLIME_TEXT_PATH));
+        applicationCommands.put(Applications.VSCODE.getDisplayName(), getEmptyIsDefault(PUSH_VSCODE_PATH));
 
         pushToApplicationPreferences = new PushToApplicationPreferences(
                 get(PUSH_TO_APPLICATION),
@@ -824,24 +824,25 @@ public PushToApplicationPreferences getPushToApplicationPreferences() {
 
     private void storePushToApplicationPath(Map<String, String> commandPair) {
         commandPair.forEach((key, value) -> {
-            switch (key) {
-                case PushToApplications.EMACS ->
+            // is only for the preferences and therefore is okay to throw NoSuchElementException
+            switch (Applications.getApplicationByDisplayName(key).get()) {
+                case Applications.EMACS ->
                         put(PUSH_EMACS_PATH, value);
-                case PushToApplications.LYX ->
+                case Applications.LYX ->
                         put(PUSH_LYXPIPE, value);
-                case PushToApplications.TEXMAKER ->
+                case Applications.TEXMAKER ->
                         put(PUSH_TEXMAKER_PATH, value);
-                case PushToApplications.TEXSTUDIO ->
+                case Applications.TEXSTUDIO ->
                         put(PUSH_TEXSTUDIO_PATH, value);
-                case PushToApplications.TEXWORKS ->
+                case Applications.TEXWORKS ->
                         put(PUSH_TEXWORKS_PATH, value);
-                case PushToApplications.VIM ->
+                case Applications.VIM ->
                         put(PUSH_VIM, value);
-                case PushToApplications.WIN_EDT ->
+                case Applications.WIN_EDT ->
                         put(PUSH_WINEDT_PATH, value);
-                case PushToApplications.SUBLIME_TEXT ->
+                case Applications.SUBLIME_TEXT ->
                         put(PUSH_SUBLIME_TEXT_PATH, value);
-                case PushToApplications.VSCODE ->
+                case Applications.VSCODE ->
                         put(PUSH_VSCODE_PATH, value);
             }
         });

jablib/src/main/java/org/jabref/logic/push/AbstractPushToApplication.java

@@ -3,11 +3,11 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.List;
+import java.util.Optional;
 
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.os.OS;
 import org.jabref.logic.util.NotificationService;
-import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.strings.StringUtil;
 
@@ -35,13 +35,32 @@ public AbstractPushToApplication(NotificationService notificationService, PushTo
         this.preferences = preferences;
     }
 
-    @Override
-    public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keyString) {
-        pushEntries(database, entries, keyString, new ProcessBuilder());
+    protected String getKeyString(List<BibEntry> entries, String delimiter) {
+        StringBuilder result = new StringBuilder();
+        Optional<String> citeKey;
+        boolean first = true;
+        for (BibEntry bes : entries) {
+            citeKey = bes.getCitationKey();
+            if (citeKey.isEmpty() || citeKey.get().isEmpty()) {
+                LOGGER.warn("Should never occur, because we made sure that all entries have keys");
+                continue;
+            }
+            if (first) {
+                result.append(citeKey.get());
+                first = false;
+            } else {
+                result.append(delimiter).append(citeKey.get());
+            }
+        }
+        return result.toString();
+    }
+
+    public void pushEntries(List<BibEntry> entries) {
+        pushEntries(entries, new ProcessBuilder());
     }
 
     @VisibleForTesting
-    public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keyString, ProcessBuilder processBuilder) {
+    public void pushEntries(List<BibEntry> entries, ProcessBuilder processBuilder) {
         couldNotPush = false;
         couldNotCall = false;
         notDefined = false;
@@ -54,6 +73,8 @@ public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, Str
             return;
         }
 
+        String keyString = this.getKeyString(entries, getDelimiter());
+
         // Execute command
         try {
             if (OS.OS_X) {

jablib/src/main/java/org/jabref/logic/push/Applications.java

@@ -0,0 +1,51 @@
+package org.jabref.logic.push;
+
+import java.util.Optional;
+
+public enum Applications {
+
+    EMACS("emacs", "Emacs"),
+    LYX("lyx", "LyX/Kile"),
+    TEXMAKER("texmaker", "Texmaker"),
+    TEXSTUDIO("texstudio", "TeXstudio"),
+    TEXWORKS("texworks", "TeXworks"),
+    VIM("vim", "Vim"),
+    WIN_EDT("winedt", "WinEdt"),
+    SUBLIME_TEXT("sublime", "Sublime Text"),
+    TEXSHOP("texshop", "TeXShop"),
+    VSCODE("vscode", "VScode");
+
+    private final String id;
+    private final String displayName;
+
+    Applications(String id, String displayName) {
+        this.id = id;
+        this.displayName = displayName;
+    }
+
+    public static Optional<Applications> getApplicationByDisplayName(String key) {
+        for (Applications application : Applications.values()) {
+            if (application.getDisplayName().equalsIgnoreCase(key)) {
+                return Optional.of(application);
+            }
+        }
+        return Optional.empty();
+    }
+
+    public static Optional<Applications> getApplicationById(String key) {
+        for (Applications application : Applications.values()) {
+            if (application.getId().equalsIgnoreCase(key)) {
+                return Optional.of(application);
+            }
+        }
+        return Optional.empty();
+    }
+
+    public String getId() {
+        return id;
+    }
+
+    public String getDisplayName() {
+        return displayName;
+    }
+}

jablib/src/main/java/org/jabref/logic/push/PushToApplication.java

@@ -3,7 +3,6 @@
 import java.nio.file.Path;
 import java.util.List;
 
-import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 
 /**
@@ -23,7 +22,7 @@ public interface PushToApplication {
      * The actual operation. This method will not be called on the event dispatch thread, so it should not do GUI
      * operations without utilizing invokeLater().
      */
-    void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keyString);
+    void pushEntries(List<BibEntry> entries);
 
     /**
      * Reporting etc., this method is called on the event dispatch thread after pushEntries() returns.

jablib/src/main/java/org/jabref/logic/push/PushToApplications.java

@@ -1,53 +1,28 @@
 package org.jabref.logic.push;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
 import java.util.Optional;
 
 import org.jabref.logic.util.NotificationService;
 
 public class PushToApplications {
 
-    public static final String EMACS = "Emacs";
-    public static final String LYX = "LyX/Kile";
-    public static final String TEXMAKER = "Texmaker";
-    public static final String TEXSTUDIO = "TeXstudio";
-    public static final String TEXWORKS = "TeXworks";
-    public static final String VIM = "Vim";
-    public static final String WIN_EDT = "WinEdt";
-    public static final String SUBLIME_TEXT = "Sublime Text";
-    public static final String TEXSHOP = "TeXShop";
-    public static final String VSCODE = "VScode";
-
-    private static final List<PushToApplication> APPLICATIONS = new ArrayList<>();
-
     private PushToApplications() {
     }
 
-    public static List<PushToApplication> getAllApplications(NotificationService notificationService, PushToApplicationPreferences preferences) {
-        if (!APPLICATIONS.isEmpty()) {
-            return Collections.unmodifiableList(APPLICATIONS);
-        }
-
-        APPLICATIONS.addAll(List.of(
-                new PushToEmacs(notificationService, preferences),
-                new PushToLyx(notificationService, preferences),
-                new PushToSublimeText(notificationService, preferences),
-                new PushToTexmaker(notificationService, preferences),
-                new PushToTeXstudio(notificationService, preferences),
-                new PushToTeXworks(notificationService, preferences),
-                new PushToVim(notificationService, preferences),
-                new PushToWinEdt(notificationService, preferences),
-                new PushToTexShop(notificationService, preferences),
-                new PushToVScode(notificationService, preferences)));
-
-        return APPLICATIONS;
-    }
-
-    public static Optional<PushToApplication> getApplicationByName(String applicationName, NotificationService notificationService, PushToApplicationPreferences preferences) {
-        return getAllApplications(notificationService, preferences).stream()
-                                                             .filter(application -> application.getDisplayName().equals(applicationName))
-                                                             .findAny();
+    /// @param applicationId Used by the CLI to select the application to run. Example: `texstudio`
+    public static Optional<PushToApplication> getApplicationById(String applicationId, NotificationService notificationService, PushToApplicationPreferences preferences) {
+        return Applications.getApplicationById(applicationId)
+                           .map(application -> switch (application) {
+                               case EMACS -> new PushToEmacs(notificationService, preferences);
+                               case LYX -> new PushToLyx(notificationService, preferences);
+                               case SUBLIME_TEXT -> new PushToSublimeText(notificationService, preferences);
+                               case TEXMAKER -> new PushToTexmaker(notificationService, preferences);
+                               case TEXSTUDIO -> new PushToTeXstudio(notificationService, preferences);
+                               case TEXWORKS -> new PushToTeXworks(notificationService, preferences);
+                               case VIM -> new PushToVim(notificationService, preferences);
+                               case WIN_EDT -> new PushToWinEdt(notificationService, preferences);
+                               case TEXSHOP -> new PushToTexShop(notificationService, preferences);
+                               case VSCODE -> new PushToVScode(notificationService, preferences);
+                           });
     }
 }