Removed swing from default file dir detection (JabRef#9222)

* Save files again relative to bib file

* Reworded

* Reintroduced default file chooser directory without swing

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update NativeDesktop.java

* Reworked ADR

* Added mac directory

* Removed comments

* Added linux support

* Revert "Save files again relative to bib file"

This reverts commit 3e640f5.

# Conflicts:
#	CHANGELOG.md
#	src/main/java/org/jabref/preferences/FilePreferences.java
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
#	src/test/java/org/jabref/model/database/BibDatabaseContextTest.java

* Update src/main/java/org/jabref/gui/desktop/os/Windows.java

* fix merge errors

* fix wrong method call

* remove defautl
fix checkstyle

* reintroduce method to avoid calling get two times

* Introduced getPath method

* Update src/main/java/org/jabref/gui/desktop/os/Linux.java

fix env var

Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: Siedlerchr <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -38,7 +38,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - A user can now add arbitrary data into `study.yml`. JabRef just ignores this data. [#9124](https://github.com/JabRef/jabref/pull/9124)
 - We reworked the External Changes Resolver dialog. [#9021](https://github.com/JabRef/jabref/pull/9021)
 - The fallback directory of the file folder now is the general file directory. In case there was a directory configured for a library and this directory was not found, JabRef placed the PDF next to the .bib file and not into the general file directory.
-- The global default directory for storing PDFs is now the subdirectory "JabRef" in the user's home.
+- The global default directory for storing PDFs is now the documents folder in the user's home.
 - We reworked the Define study parameters dialog. [#9123](https://github.com/JabRef/jabref/pull/9123)
 - We simplified the actions to fast-resolve duplicates to 'Keep Left', 'Keep Right', 'Keep Both' and 'Keep Merged'. [#9056](https://github.com/JabRef/jabref/issues/9056)
 - We fixed an issue where a message about changed metadata would occur on saving although nothing changed. [#9159](https://github.com/JabRef/jabref/issues/9159)

docs/decisions/0026-use-swings-file-chooser-to-determine-default-directory.md renamed to docs/decisions/0026-use-jna-to-determine-default-directory.md

@@ -2,7 +2,7 @@
 nav_order: 26
 parent: Decision Records
 ---
-# Use Swing's FileChooser to Determine Default Directory
+# Use Java Native Access to Determine Default Directory
 
 ## Context and Problem Statement
 
@@ -20,11 +20,11 @@ How to determine the "best" directory native for the OS the user runs.
 * Use Swing's FileChooser to Determine Default Directory
 * Use `user.home`
 * [AppDirs](https://github.com/harawata/appdirs)
+* [Java Native Access](https://github.com/java-native-access/jna)
 
 ## Decision Outcome
 
-Chosen option: "Use Swing's FileChooser to Determine Default Directory", because
-comes out best (see below).
+Chosen option: "Java Native Access", because comes out best (see below).
 
 ## Pros and Cons of the Options
 
@@ -51,7 +51,13 @@ There is `System.getProperty("user.home");`.
 > AppDirs is a small java library which provides a path to the platform dependent special folder/directory.
 
 * Good, because already used in JabRef
-* Bad, because does not use "MyDocuments" on Windows, but rather `C:\Users\<Account>\AppData\<AppAuthor>\<AppName>` as basis
+* Bad, because does not use `Documents` on Windows, but rather `C:\Users\<Account>\AppData\<AppAuthor>\<AppName>` as basis
+
+### Java Native Access
+
+* Good, because no additional dependency required, as it is already loaded by AppDirs
+* Good, because it is well maintained and widely used
+* Good, because it provides direct access to `Documents` and other system variables
 
 ## More Information
 

src/main/java/module-info.java

@@ -114,6 +114,7 @@
     requires com.fasterxml.jackson.dataformat.yaml;
     requires com.fasterxml.jackson.datatype.jsr310;
     requires net.harawata.appdirs;
+    requires com.sun.jna.platform;
 
     requires org.eclipse.jgit;
     requires com.jthemedetector;

src/main/java/org/jabref/gui/desktop/JabRefDesktop.java

@@ -10,9 +10,6 @@
 import java.util.Optional;
 import java.util.regex.Pattern;
 
-import javax.swing.filechooser.FileSystemView;
-
-import org.jabref.architecture.AllowedToUseSwing;
 import org.jabref.gui.DialogService;
 import org.jabref.gui.Globals;
 import org.jabref.gui.desktop.os.DefaultDesktop;
@@ -40,7 +37,6 @@
  * TODO: Replace by http://docs.oracle.com/javase/7/docs/api/java/awt/Desktop.html
  * http://stackoverflow.com/questions/18004150/desktop-api-is-not-supported-on-the-current-platform
  */
-@AllowedToUseSwing("Needs access to swing for the user's os dependent file chooser path")
 public class JabRefDesktop {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(JabRefDesktop.class);
@@ -292,21 +288,6 @@ public static void openConsole(File file, PreferencesService preferencesService,
         }
     }
 
-    /**
-     * Get the user's default file chooser directory
-     *
-     * @return The path to the directory
-     */
-    public static String getDefaultFileChooserDirectory() {
-        // Implementation: ADR-0026
-
-        // We do not return a subdirectory "JabRef", because
-        //   - the directory might not exist at this point of the method
-        //   - we might not have the rights to create a directory
-        //   - getters should not have any side effect
-        return FileSystemView.getFileSystemView().getDefaultDirectory().getPath();
-    }
-
     // TODO: Move to OS.java
     public static NativeDesktop getNativeDesktop() {
         if (OS.WINDOWS) {

src/main/java/org/jabref/gui/desktop/os/DefaultDesktop.java

@@ -45,4 +45,9 @@ public String detectProgramPath(String programName, String directoryName) {
     public Path getApplicationDirectory() {
         return getUserDirectory();
     }
+
+    @Override
+    public Path getDefaultFileChooserDirectory() {
+        return Path.of(System.getProperty("user.home"));
+    }
 }

src/main/java/org/jabref/gui/desktop/os/Linux.java

@@ -8,6 +8,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.Optional;
 
 import org.jabref.architecture.AllowedToUseAwt;
@@ -134,7 +135,7 @@ public void openConsole(String absolutePath, DialogService dialogService) throws
             if (emulatorName != null) {
                 emulatorName = emulatorName.substring(emulatorName.lastIndexOf(File.separator) + 1);
 
-                String[] cmd = {};
+                String[] cmd;
                 if (emulatorName.contains("gnome")) {
                     cmd = new String[] {"gnome-terminal", "--working-directory=", absolutePath};
                 } else if (emulatorName.contains("xfce4")) {
@@ -167,4 +168,12 @@ public String detectProgramPath(String programName, String directoryName) {
     public Path getApplicationDirectory() {
         return Path.of("/usr/lib/");
     }
+
+    @Override
+    public Path getDefaultFileChooserDirectory() {
+        return Path.of(Objects.requireNonNullElse(
+                System.getenv("XDG_DOCUMENTS_DIR"),
+                System.getProperty("user.home") + "/Documents")
+        );
+    }
 }

src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java

@@ -14,7 +14,6 @@ public interface NativeDesktop {
      *
      * @param filePath    The filename.
      * @param application Link to the app that opens the file.
-     * @throws IOException
      */
     void openFileWithApplication(String filePath, String application) throws IOException;
 
@@ -31,6 +30,13 @@ public interface NativeDesktop {
      */
     Path getApplicationDirectory();
 
+    /**
+     * Get the user's default file chooser directory
+     *
+     * @return The path to the directory
+     */
+     Path getDefaultFileChooserDirectory();
+
     /**
      * Returns the path to the system's user directory.
      *

src/main/java/org/jabref/gui/desktop/os/OSX.java

@@ -52,4 +52,9 @@ public String detectProgramPath(String programName, String directoryName) {
     public Path getApplicationDirectory() {
         return Path.of("/Applications");
     }
+
+    @Override
+    public Path getDefaultFileChooserDirectory() {
+        return Path.of(System.getProperty("user.home") + "/Documents");
+    }
 }

src/main/java/org/jabref/gui/desktop/os/Windows.java

@@ -11,7 +11,16 @@
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 
+import com.sun.jna.platform.win32.KnownFolders;
+import com.sun.jna.platform.win32.Shell32Util;
+import com.sun.jna.platform.win32.ShlObj;
+import com.sun.jna.platform.win32.Win32Exception;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class Windows implements NativeDesktop {
+    private static final Logger LOGGER = LoggerFactory.getLogger(Windows.class);
+
     private static final String DEFAULT_EXECUTABLE_EXTENSION = ".exe";
 
     @Override
@@ -70,6 +79,21 @@ public Path getApplicationDirectory() {
         return getUserDirectory();
     }
 
+    @Override
+    public Path getDefaultFileChooserDirectory() {
+        try {
+            try {
+                return Path.of(Shell32Util.getKnownFolderPath(KnownFolders.FOLDERID_Documents));
+            } catch (UnsatisfiedLinkError e) {
+                // Windows Vista or earlier
+                return Path.of(Shell32Util.getFolderPath(ShlObj.CSIDL_MYDOCUMENTS));
+            }
+        } catch (Win32Exception e) {
+            LOGGER.error("Error accessing folder", e);
+            return Path.of(System.getProperty("user.home"));
+        }
+    }
+
     @Override
     public void openFileWithApplication(String filePath, String application) throws IOException {
         new ProcessBuilder(Path.of(application).toString(), Path.of(filePath).toString()).start();

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java

@@ -74,7 +74,7 @@ public LinkedFilesTabViewModel(DialogService dialogService, PreferencesService p
     @Override
     public void setValues() {
         // External files preferences / Attached files preferences / File preferences
-        mainFileDirectoryProperty.setValue(filePreferences.getFileDirectory().orElse(Path.of("")).toString());
+        mainFileDirectoryProperty.setValue(filePreferences.getMainFileDirectory().orElse(Path.of("")).toString());
         useMainFileDirectoryProperty.setValue(!filePreferences.shouldStoreFilesRelativeToBibFile());
         useBibLocationAsPrimaryProperty.setValue(filePreferences.shouldStoreFilesRelativeToBibFile());
         fulltextIndex.setValue(filePreferences.shouldFulltextIndexLinkedFiles());

src/main/java/org/jabref/model/database/BibDatabaseContext.java

@@ -171,7 +171,7 @@ public List<Path> getFileDirectories(FilePreferences preferences) {
             });
         } else {
             // Main file directory
-            preferences.getFileDirectory().ifPresent(fileDirs::add);
+            preferences.getMainFileDirectory().ifPresent(fileDirs::add);
         }
 
         return fileDirs.stream().map(Path::toAbsolutePath).collect(Collectors.toList());

src/main/java/org/jabref/preferences/FilePreferences.java

@@ -56,7 +56,7 @@ public String getUser() {
         return user.getValue();
     }
 
-    public Optional<Path> getFileDirectory() {
+    public Optional<Path> getMainFileDirectory() {
         if (StringUtil.isBlank(mainFileDirectory.getValue())) {
             return Optional.empty();
         } else {

src/main/java/org/jabref/preferences/JabRefPreferences.java

@@ -949,6 +949,14 @@ public List<String> getStringList(String key) {
         return convertStringToList(get(key));
     }
 
+    /**
+     * Returns a Path
+     */
+    private Path getPath(String key, Path defaultValue) {
+        String rawPath = get(key);
+        return StringUtil.isNotBlank(rawPath) ? Path.of(rawPath) : defaultValue;
+    }
+
     /**
      * Clear all preferences.
      *
@@ -2186,20 +2194,6 @@ public FieldWriterPreferences getFieldWriterPreferences() {
                 getFieldContentParserPreferences());
     }
 
-    /**
-     * Ensures that the main file directory is a non-empty String.
-     * The directory is <emph>NOT</emph> created, because creation of the directory is the task of the respective methods.
-     *
-     * @param originalDirectory the directory as configured
-     */
-    private String determineMainFileDirectory(String originalDirectory) {
-        if ((originalDirectory != null) && !originalDirectory.isEmpty()) {
-            // A non-empty directory is kept
-            return originalDirectory;
-        }
-        return JabRefDesktop.getDefaultFileChooserDirectory();
-    }
-
     @Override
     public FilePreferences getFilePreferences() {
         if (Objects.nonNull(filePreferences)) {
@@ -2208,7 +2202,7 @@ public FilePreferences getFilePreferences() {
 
         filePreferences = new FilePreferences(
                 getInternalPreferences().getUser(),
-                determineMainFileDirectory(get(MAIN_FILE_DIRECTORY)),
+                getPath(MAIN_FILE_DIRECTORY, JabRefDesktop.getNativeDesktop().getDefaultFileChooserDirectory()).toString(),
                 getBoolean(STORE_RELATIVE_TO_BIB),
                 get(IMPORT_FILENAMEPATTERN),
                 get(IMPORT_FILEDIRPATTERN),

src/test/java/org/jabref/logic/pdf/search/indexing/DocumentReaderTest.java

@@ -1,6 +1,5 @@
 package org.jabref.logic.pdf.search.indexing;
 
-import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
@@ -35,12 +34,12 @@ public void setup() {
         when(databaseContext.getFileDirectories(Mockito.any())).thenReturn(Collections.singletonList(Path.of("src/test/resources/pdfs")));
         this.filePreferences = mock(FilePreferences.class);
         when(filePreferences.getUser()).thenReturn("test");
-        when(filePreferences.getFileDirectory()).thenReturn(Optional.empty());
+        when(filePreferences.getMainFileDirectory()).thenReturn(Optional.empty());
         when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(true);
     }
 
     @Test
-    public void unknownFileTestShouldReturnEmptyList() throws IOException {
+    public void unknownFileTestShouldReturnEmptyList() {
         // given
         BibEntry entry = new BibEntry();
         entry.setFiles(Collections.singletonList(new LinkedFile("Wrong path", "NOT_PRESENT.pdf", "Type")));

src/test/java/org/jabref/model/database/BibDatabaseContextTest.java

@@ -94,10 +94,9 @@ void getFileDirectoriesWithMetadata() {
     @Test
     void getUserFileDirectoryIfAllAreEmpty() {
         when(fileDirPrefs.shouldStoreFilesRelativeToBibFile()).thenReturn(false);
+        Path userDirJabRef = JabRefDesktop.getNativeDesktop().getDefaultFileChooserDirectory();
 
-        Path userDirJabRef = Path.of(JabRefDesktop.getDefaultFileChooserDirectory(), "JabRef");
-
-        when(fileDirPrefs.getFileDirectory()).thenReturn(Optional.of(userDirJabRef));
+        when(fileDirPrefs.getMainFileDirectory()).thenReturn(Optional.of(userDirJabRef));
         BibDatabaseContext database = new BibDatabaseContext();
         database.setDatabasePath(Path.of("biblio.bib"));
         assertEquals(Collections.singletonList(userDirJabRef), database.getFileDirectories(fileDirPrefs));