diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b701f44d02..830fb823352 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/docs/decisions/0026-use-swings-file-chooser-to-determine-default-directory.md b/docs/decisions/0026-use-jna-to-determine-default-directory.md similarity index 77% rename from docs/decisions/0026-use-swings-file-chooser-to-determine-default-directory.md rename to docs/decisions/0026-use-jna-to-determine-default-directory.md index 127067b12f8..40b76f03e39 100644 --- a/docs/decisions/0026-use-swings-file-chooser-to-determine-default-directory.md +++ b/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\\AppData\\` as basis +* Bad, because does not use `Documents` on Windows, but rather `C:\Users\\AppData\\` 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 diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 4edbf562d8c..fbc9f463f2d 100644 --- a/src/main/java/module-info.java +++ b/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; uses org.eclipse.jgit.transport.SshSessionFactory; diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java index cb43b1284ab..517ffc1689d 100644 --- a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java +++ b/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) { diff --git a/src/main/java/org/jabref/gui/desktop/os/DefaultDesktop.java b/src/main/java/org/jabref/gui/desktop/os/DefaultDesktop.java index ec0805b641f..86d97fb45ae 100644 --- a/src/main/java/org/jabref/gui/desktop/os/DefaultDesktop.java +++ b/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")); + } } diff --git a/src/main/java/org/jabref/gui/desktop/os/Linux.java b/src/main/java/org/jabref/gui/desktop/os/Linux.java index 289efb4f576..c08ca68aa2c 100644 --- a/src/main/java/org/jabref/gui/desktop/os/Linux.java +++ b/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") + ); + } } diff --git a/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java b/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java index 349368ef3a6..74341d10e8a 100644 --- a/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java +++ b/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. * diff --git a/src/main/java/org/jabref/gui/desktop/os/OSX.java b/src/main/java/org/jabref/gui/desktop/os/OSX.java index d7a3c6cd710..ece1d553d56 100644 --- a/src/main/java/org/jabref/gui/desktop/os/OSX.java +++ b/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"); + } } diff --git a/src/main/java/org/jabref/gui/desktop/os/Windows.java b/src/main/java/org/jabref/gui/desktop/os/Windows.java index d85722740d6..eb973986e0e 100644 --- a/src/main/java/org/jabref/gui/desktop/os/Windows.java +++ b/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(); diff --git a/src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java b/src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java index d04d90e8f54..ea38ed9f858 100644 --- a/src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java +++ b/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()); diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 4331f66d90b..436f2b070c7 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -171,7 +171,7 @@ public List 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()); diff --git a/src/main/java/org/jabref/preferences/FilePreferences.java b/src/main/java/org/jabref/preferences/FilePreferences.java index 219fe1c9d49..264d778aeda 100644 --- a/src/main/java/org/jabref/preferences/FilePreferences.java +++ b/src/main/java/org/jabref/preferences/FilePreferences.java @@ -56,7 +56,7 @@ public String getUser() { return user.getValue(); } - public Optional getFileDirectory() { + public Optional getMainFileDirectory() { if (StringUtil.isBlank(mainFileDirectory.getValue())) { return Optional.empty(); } else { diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 166e5d9f187..f4ddffdb5bf 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -944,6 +944,14 @@ public List 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. * @@ -2179,20 +2187,6 @@ public FieldWriterPreferences getFieldWriterPreferences() { getFieldContentParserPreferences()); } - /** - * Ensures that the main file directory is a non-empty String. - * The directory is NOT 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)) { @@ -2201,7 +2195,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), diff --git a/src/test/java/org/jabref/logic/pdf/search/indexing/DocumentReaderTest.java b/src/test/java/org/jabref/logic/pdf/search/indexing/DocumentReaderTest.java index 27cdb271844..047689eb543 100644 --- a/src/test/java/org/jabref/logic/pdf/search/indexing/DocumentReaderTest.java +++ b/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"))); diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 181117f7dac..4f6a3335f48 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/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));