From fcf678e23a41e23931137a8bcdaedfc08cf5c516 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:17:41 +0200 Subject: [PATCH 01/19] Fix typos --- CHANGELOG.md | 2 +- .../java/org/jabref/gui/exporter/ExportToClipboardAction.java | 2 +- src/main/java/org/jabref/preferences/JabRefPreferences.java | 2 +- src/main/java/org/jabref/preferences/PreferencesService.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab6ce7ec3a..e3d6a77a5ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed a performance regression when opening large libraries [#9041](https://github.com/JabRef/jabref/issues/9041) - We fixed a bug where spaces are trimmed when highlighting differences in the Entries merge dialog. [koppor#371](https://github.com/koppor/jabref/issues/371) - We fixed several bugs regarding the manual and the autosave of library files that sometimes lead to exceptions or data loss. [#9067](https://github.com/JabRef/jabref/pull/9067), [#8448](https://github.com/JabRef/jabref/issues/8484), [#8746](https://github.com/JabRef/jabref/issues/8746), [#6684](https://github.com/JabRef/jabref/issues/6684), [#6644](https://github.com/JabRef/jabref/issues/6644), [#6102](https://github.com/JabRef/jabref/issues/6102), [#6002](https://github.com/JabRef/jabref/issues/6000) -- We fixed an issue where applied save actions on saving the library file would lead to the dialog "The libary has been modified by another program" popping up [#4877](https://github.com/JabRef/jabref/issues/4877) +- We fixed an issue where applied save actions on saving the library file would lead to the dialog "The library has been modified by another program" popping up [#4877](https://github.com/JabRef/jabref/issues/4877) - We fixed an issue where title case didn't capitalize words after en-dash characters [#9068] - We fixed an issue where JabRef would not exit when a connection to a LibreOffice document was established previously and the document is still open [#9075](https://github.com/JabRef/jabref/issues/9075) diff --git a/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java b/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java index 3407c25c153..1bf864a1c76 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java +++ b/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java @@ -103,7 +103,7 @@ private ExportResult exportToClipboard(Exporter exporter) throws Exception { // Set the global variable for this database's file directory before exporting, // so formatters can resolve linked files correctly. // (This is an ugly hack!) - preferences.storeFileDirforDatabase(stateManager.getActiveDatabase() + preferences.storeFileDirForDatabase(stateManager.getActiveDatabase() .map(db -> db.getFileDirectories(preferences.getFilePreferences())) .orElse(List.of(preferences.getFilePreferences().getWorkingDirectory()))); diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 1ec582152f6..9d7aaa2041a 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1124,7 +1124,7 @@ public FileLinkPreferences getFileLinkPreferences() { } @Override - public void storeFileDirforDatabase(List dirs) { + public void storeFileDirForDatabase(List dirs) { this.fileDirForDatabase = dirs; } diff --git a/src/main/java/org/jabref/preferences/PreferencesService.java b/src/main/java/org/jabref/preferences/PreferencesService.java index f3329750992..eed949f5914 100644 --- a/src/main/java/org/jabref/preferences/PreferencesService.java +++ b/src/main/java/org/jabref/preferences/PreferencesService.java @@ -221,7 +221,7 @@ public interface PreferencesService { FileLinkPreferences getFileLinkPreferences(); - void storeFileDirforDatabase(List dirs); + void storeFileDirForDatabase(List dirs); //************************************************************************************************************* // Import/Export preferences From 4143b1d3bd7b6891926039f4b05f8a3db9e659d8 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:18:11 +0200 Subject: [PATCH 02/19] Add comment --- .../jabref/logic/layout/format/FileLinkPreferences.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/java/org/jabref/logic/layout/format/FileLinkPreferences.java b/src/main/java/org/jabref/logic/layout/format/FileLinkPreferences.java index 805c4f5db76..20b9dcd067e 100644 --- a/src/main/java/org/jabref/logic/layout/format/FileLinkPreferences.java +++ b/src/main/java/org/jabref/logic/layout/format/FileLinkPreferences.java @@ -17,6 +17,15 @@ public String getMainFileDirectory() { return mainFileDirectory; } + /** + * The following field is used as a global variable during the export of a database. + * By setting this field to the path of the database's default file directory, formatters + * that should resolve external file paths can access this field. This is an ugly hack + * to solve the problem of formatters not having access to any context except for the + * string to be formatted and possible formatter arguments. + * + * See also {@link org.jabref.preferences.JabRefPreferences#fileDirForDatabase} + */ public List getFileDirForDatabase() { return fileDirForDatabase; } From 44625f9f593bb01d2f1404999627e843f610e0ff Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:18:47 +0200 Subject: [PATCH 03/19] FileLinkPreferences now rely on FilePreferences --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 9d7aaa2041a..4776f9bf935 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -410,6 +410,7 @@ public class JabRefPreferences implements PreferencesService { // to solve the problem of formatters not having access to any context except for the // string to be formatted and possible formatter arguments. public List fileDirForDatabase; + private final Preferences prefs; /** @@ -1119,7 +1120,7 @@ public void importPreferences(Path file) throws JabRefException { @Override public FileLinkPreferences getFileLinkPreferences() { return new FileLinkPreferences( - get(MAIN_FILE_DIRECTORY), // REALLY HERE? + getFilePreferences().mainFileDirectoryProperty().get(), fileDirForDatabase); } From 7406d495ef97b10a74ab846de383a1e561b5d8dd Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:19:23 +0200 Subject: [PATCH 04/19] The fallback directory of the file folder now is the general file directory. Co-authored-by: Kevin Klein --- CHANGELOG.md | 2 +- .../jabref/model/database/BibDatabaseContext.java | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3d6a77a5ea..a07d64dba90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - JabRef keeps 10 older versions of a `.bib` file in the [user data dir](https://github.com/harawata/appdirs#supported-directories) (instead of a single `.sav` (now: `.bak`) file in the directory of the `.bib` file) - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action. - 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. ### Fixed diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 662a1ecde36..b184b7dca49 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -115,15 +115,20 @@ public boolean isBiblatexMode() { /** * Look up the directories set up for this database. - * There can be up to three directory definitions for these files: the database's - * metadata can specify a general directory and/or a user-specific directory, or the preferences can specify one. + * There can be up to four directories definitions for these files: + *
    + *
  1. next to the .bib file.
    2. + *
  2. the preferences can specify a default one.
    3. + *
  3. the database's metadata can specify a general directory.
    4. + *
  4. the database's metadata can specify a user-specific directory.
    5. + *
*

* The settings are prioritized in the following order, and the first defined setting is used: *

    *
  1. user-specific metadata directory
    2. *
  2. general metadata directory
    3. - *
  3. preferences directory
    4. - *
  4. BIB file directory
    5. + *
  5. BIB file directory (if configured in the preferences AND none of the two above directories are configured)
    6. + *
  6. preferences directory (if .bib file directory should not be used according to the preferences)
    7. *
* * @param preferences The fileDirectory preferences @@ -140,7 +145,7 @@ public List getFileDirectories(FilePreferences preferences) { .ifPresent(metaDataDirectory -> fileDirs.add(getFileDirectoryPath(metaDataDirectory))); // 3. BIB file directory or Main file directory - if (preferences.shouldStoreFilesRelativeToBibFile()) { + if (fileDirs.isEmpty() && preferences.shouldStoreFilesRelativeToBibFile()) { getDatabasePath().ifPresent(dbPath -> { Path parentPath = dbPath.getParent(); if (parentPath == null) { From 963a3a69defda3008559335a954baa32b57beaf7 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:24:28 +0200 Subject: [PATCH 05/19] Improve LOGGER call with {} --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 4776f9bf935..d4f78a4d43f 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -2795,7 +2795,7 @@ private Set getCustomImportFormats() { importers.add(new CustomImporter(importerString.get(3), importerString.get(2))); } } catch (Exception e) { - LOGGER.warn("Could not load " + importerString.get(0) + " from preferences. Will ignore.", e); + LOGGER.warn("Could not load {} from preferences. Will ignore.", importerString.get(0), e); } } From 75e668000ea1e9c90e1a69e69970a288b99e7e57 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:50:54 +0200 Subject: [PATCH 06/19] The global default directory for storing PDFs is now the subdirectory "JabRef" in the user's home Co-authored-by: Kevin Klein --- CHANGELOG.md | 1 + .../jabref/preferences/FilePreferences.java | 2 +- .../jabref/preferences/JabRefPreferences.java | 23 ++++++++++++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a07d64dba90..894cb1aaa9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action. - 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. ### Fixed diff --git a/src/main/java/org/jabref/preferences/FilePreferences.java b/src/main/java/org/jabref/preferences/FilePreferences.java index 037324fc1dd..219fe1c9d49 100644 --- a/src/main/java/org/jabref/preferences/FilePreferences.java +++ b/src/main/java/org/jabref/preferences/FilePreferences.java @@ -52,7 +52,7 @@ public FilePreferences(String user, this.externalFileTypes.addAll(externalFileTypes); } - public String getUser() { // Read only + public String getUser() { return user.getValue(); } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index d4f78a4d43f..77b858d70a5 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -33,6 +33,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.swing.filechooser.FileSystemView; + import javafx.beans.InvalidationListener; import javafx.collections.SetChangeListener; import javafx.scene.control.TableColumn.SortType; @@ -2208,6 +2210,25 @@ 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.isEmpty()) { + // A non-empty directory is kept + return originalDirectory; + } + // Property "user.home" might be "difficult" on Windows + // See https://stackoverflow.com/a/586917/873282 for a longer discussion + // The proposed solution is to use Swing's FileSystemView + // See https://stackoverflow.com/a/32914568/873282 + String userHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); + return Path.of(userHome, "JabRef").toString(); + } + @Override public FilePreferences getFilePreferences() { if (Objects.nonNull(filePreferences)) { @@ -2216,7 +2237,7 @@ public FilePreferences getFilePreferences() { filePreferences = new FilePreferences( getInternalPreferences().getUser(), - get(MAIN_FILE_DIRECTORY), + determineMainFileDirectory(get(MAIN_FILE_DIRECTORY)), getBoolean(STORE_RELATIVE_TO_BIB), get(IMPORT_FILENAMEPATTERN), get(IMPORT_FILEDIRPATTERN), From bce2253a0463dee6145b30638f4b0270f98bb4f2 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 1 Sep 2022 23:54:35 +0200 Subject: [PATCH 07/19] Adapt tests to new behavior Co-authored-by: Kevin Klein --- .../org/jabref/model/database/BibDatabaseContextTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 7c8e52fa033..800af3896f0 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -1,8 +1,8 @@ package org.jabref.model.database; import java.nio.file.Path; -import java.util.Arrays; import java.util.Collections; +import java.util.List; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.types.IEEETranEntryType; @@ -70,7 +70,7 @@ void getFileDirectoriesWithRelativeMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("../Literature"); - assertEquals(Arrays.asList(Path.of("/absolute/Literature").toAbsolutePath(), currentWorkingDir.resolve(file.getParent())), + assertEquals(List.of(Path.of("/absolute/Literature").toAbsolutePath()), database.getFileDirectories(fileDirPrefs)); } @@ -81,7 +81,7 @@ void getFileDirectoriesWithMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("Literature"); - assertEquals(Arrays.asList(Path.of("/absolute/subdir/Literature").toAbsolutePath(), currentWorkingDir.resolve(file.getParent())), + assertEquals(List.of(Path.of("/absolute/subdir/Literature").toAbsolutePath()), database.getFileDirectories(fileDirPrefs)); } From c9656ff43d6b6a227aa185575bffd7a15e4aafa8 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 00:56:51 +0200 Subject: [PATCH 08/19] Refine comment --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 77b858d70a5..ee73a3a639d 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -2225,8 +2225,10 @@ private String determineMainFileDirectory(String originalDirectory) { // See https://stackoverflow.com/a/586917/873282 for a longer discussion // The proposed solution is to use Swing's FileSystemView // See https://stackoverflow.com/a/32914568/873282 - String userHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); - return Path.of(userHome, "JabRef").toString(); + // As of 2022, System.getProperty("user.home") returns "c:\Users\username" on Windows 10, whereas + // the FileSystemView returns "C:\Users\username\Documents", which is the "better" directory + String documentsHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); + return Path.of(documentsHome, "JabRef").toString(); } @Override From 50d73e7a4c440c0717c0ca996dce20c37b219541 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 10:34:42 +0200 Subject: [PATCH 09/19] Try to fix checkstyle --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index ee73a3a639d..626c6174f24 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -2225,8 +2225,8 @@ private String determineMainFileDirectory(String originalDirectory) { // See https://stackoverflow.com/a/586917/873282 for a longer discussion // The proposed solution is to use Swing's FileSystemView // See https://stackoverflow.com/a/32914568/873282 - // As of 2022, System.getProperty("user.home") returns "c:\Users\username" on Windows 10, whereas - // the FileSystemView returns "C:\Users\username\Documents", which is the "better" directory + // As of 2022, System.getProperty("user.home") returns c:\Users\username on Windows 10, whereas + // the FileSystemView returns C:\Users\username\Documents, which is the "better" directory String documentsHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); return Path.of(documentsHome, "JabRef").toString(); } From 3b14a86133fb8f78756e5b07ebdd8900153e4993 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 10:41:16 +0200 Subject: [PATCH 10/19] Fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7048b971ae..caf8b06c061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve ### Fixed -- We fixed an issue where the possibilty to generate a subdatabase from an aux file was writing empty files when called from the commandline [#9115](https://github.com/JabRef/jabref/issues/9115), [forum#3516](https://discourse.jabref.org/t/export-subdatabase-from-aux-file-on-macos-command-line/3516) +- We fixed an issue where the possibility to generate a subdatabase from an aux file was writing empty files when called from the commandline [#9115](https://github.com/JabRef/jabref/issues/9115), [forum#3516](https://discourse.jabref.org/t/export-subdatabase-from-aux-file-on-macos-command-line/3516) - We fixed the display of issue, number, eid and pages fields in the entry preview. [#8607](https://github.com/JabRef/jabref/pull/8607), [#8372](https://github.com/JabRef/jabref/issues/8372), [Koppor#514](https://github.com/koppor/jabref/issues/514), [forum#2390](https://discourse.jabref.org/t/unable-to-edit-my-bibtex-file-that-i-used-before-vers-5-1/2390), [forum#3462](https://discourse.jabref.org/t/jabref-5-6-need-help-with-export-from-jabref-to-microsoft-word-entry-preview-of-apa-7-not-rendering-correctly/3462) - We fixed the page ranges checker to detect article numbers in the pages field (used at [Check Integrity](https://docs.jabref.org/finding-sorting-and-cleaning-entries/checkintegrity)). [#8607](https://github.com/JabRef/jabref/pull/8607) - The [HtmlToLaTeXFormatter](https://docs.jabref.org/finding-sorting-and-cleaning-entries/saveactions#html-to-latex) keeps single `<` characters. From caf101aea21de17762dba64865b807bf0d92498e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 10:41:22 +0200 Subject: [PATCH 11/19] Fix "illegal unicode escape" --- src/main/java/org/jabref/preferences/JabRefPreferences.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 626c6174f24..11e8b1ea6a4 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -2225,8 +2225,8 @@ private String determineMainFileDirectory(String originalDirectory) { // See https://stackoverflow.com/a/586917/873282 for a longer discussion // The proposed solution is to use Swing's FileSystemView // See https://stackoverflow.com/a/32914568/873282 - // As of 2022, System.getProperty("user.home") returns c:\Users\username on Windows 10, whereas - // the FileSystemView returns C:\Users\username\Documents, which is the "better" directory + // As of 2022, System.getProperty("user.home") returns c:\Users\USERNAME on Windows 10, whereas + // the FileSystemView returns C:\Users\USERNAME\Documents, which is the "better" directory String documentsHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); return Path.of(documentsHome, "JabRef").toString(); } From eb899704b7e0faaef79deba4971f77443d481e3c Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 19:19:59 +0200 Subject: [PATCH 12/19] Add architecture test and move to java desktop remove obsolete checks --- .../org/jabref/gui/desktop/JabRefDesktop.java | 18 +++++ .../jabref/preferences/JabRefPreferences.java | 13 +--- .../architecture/MainArchitectureTests.java | 72 +++++++------------ 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java index 517ffc1689d..d32a756430a 100644 --- a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java +++ b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java @@ -10,6 +10,9 @@ 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; @@ -37,6 +40,7 @@ * 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); @@ -288,6 +292,20 @@ 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() { + // Property "user.home" might be "difficult" on Windows + // See https://stackoverflow.com/a/586917/873282 for a longer discussion + // The proposed solution is to use Swing's FileSystemView + // See https://stackoverflow.com/a/32914568/873282 + // As of 2022, System.getProperty("user.home") returns c:\Users\USERNAME on Windows 10, whereas + // the FileSystemView returns C:\Users\USERNAME\Documents, which is the "better" directory + 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/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 11e8b1ea6a4..248addc2aac 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -33,8 +33,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import javax.swing.filechooser.FileSystemView; - import javafx.beans.InvalidationListener; import javafx.collections.SetChangeListener; import javafx.scene.control.TableColumn.SortType; @@ -1478,7 +1476,7 @@ private void updateEntryEditorTabList() { List tabNames = getSeries(CUSTOM_TAB_NAME); List tabFields = getSeries(CUSTOM_TAB_FIELDS); - if (tabNames.isEmpty() || tabNames.size() != tabFields.size()) { + if (tabNames.isEmpty() || (tabNames.size() != tabFields.size())) { // Nothing set, so we use the default values tabNames = getSeries(CUSTOM_TAB_NAME + "_def"); tabFields = getSeries(CUSTOM_TAB_FIELDS + "_def"); @@ -2221,14 +2219,7 @@ private String determineMainFileDirectory(String originalDirectory) { // A non-empty directory is kept return originalDirectory; } - // Property "user.home" might be "difficult" on Windows - // See https://stackoverflow.com/a/586917/873282 for a longer discussion - // The proposed solution is to use Swing's FileSystemView - // See https://stackoverflow.com/a/32914568/873282 - // As of 2022, System.getProperty("user.home") returns c:\Users\USERNAME on Windows 10, whereas - // the FileSystemView returns C:\Users\USERNAME\Documents, which is the "better" directory - String documentsHome = FileSystemView.getFileSystemView().getDefaultDirectory().getPath(); - return Path.of(documentsHome, "JabRef").toString(); + return Path.of(JabRefDesktop.getDefaultFileChooserDirectory(), "JabRef").toString(); } @Override diff --git a/src/test/java/org/jabref/architecture/MainArchitectureTests.java b/src/test/java/org/jabref/architecture/MainArchitectureTests.java index 6aafd6acc1a..b3702e019dc 100644 --- a/src/test/java/org/jabref/architecture/MainArchitectureTests.java +++ b/src/test/java/org/jabref/architecture/MainArchitectureTests.java @@ -17,7 +17,6 @@ class MainArchitectureTests { public static final String CLASS_ORG_JABREF_GLOBALS = "org.jabref.gui.Globals"; private static final String PACKAGE_JAVAX_SWING = "javax.swing.."; private static final String PACKAGE_JAVA_AWT = "java.awt.."; - private static final String PACKAGE_JAVA_FX = "javafx.."; private static final String PACKAGE_ORG_JABREF_GUI = "org.jabref.gui.."; private static final String PACKAGE_ORG_JABREF_LOGIC = "org.jabref.logic.."; private static final String PACKAGE_ORG_JABREF_MODEL = "org.jabref.model.."; @@ -33,34 +32,17 @@ public static void doNotUseApacheCommonsLang3(JavaClasses classes) { @ArchTest public static void doNotUseSwing(JavaClasses classes) { // This checks for all all Swing packages, but not the UndoManager - noClasses().should().accessClassesThat().resideInAnyPackage("javax.swing", - "javax.swing.border..", - "javax.swing.colorchooser..", - "javax.swing.event..", - "javax.swing.filechooser..", - "javax.swing.plaf..", - "javax.swing.table..", - "javax.swing.text..", - "javax.swing.tree.." - ).check(classes); - } - - @ArchTest - public static void doNotUseJGoodies(JavaClasses classes) { - noClasses().should().accessClassesThat().resideInAPackage("com.jgoodies..") - .check(classes); - } - - @ArchTest - public static void doNotUseGlazedLists(JavaClasses classes) { - noClasses().should().accessClassesThat().resideInAPackage("ca.odell.glazedlists..") - .check(classes); - } - - @ArchTest - public static void doNotUseGlyphsDirectly(JavaClasses classes) { - noClasses().that().resideOutsideOfPackage("org.jabref.gui.icon") - .should().accessClassesThat().resideInAnyPackage("de.jensd.fx.glyphs", "de.jensd.fx.glyphs.materialdesignicons") + noClasses().that().areNotAnnotatedWith(AllowedToUseSwing.class) + .should().accessClassesThat() + .resideInAnyPackage("javax.swing", + "javax.swing.border..", + "javax.swing.colorchooser..", + "javax.swing.event..", + "javax.swing.filechooser..", + "javax.swing.plaf..", + "javax.swing.table..", + "javax.swing.text..", + "javax.swing.tree..") .check(classes); } @@ -91,22 +73,22 @@ public static void doNotUsePaths(JavaClasses classes) { // Fails currently public static void respectLayeredArchitecture(JavaClasses classes) { layeredArchitecture() - .layer("Gui").definedBy(PACKAGE_ORG_JABREF_GUI) - .layer("Logic").definedBy(PACKAGE_ORG_JABREF_LOGIC) - .layer("Model").definedBy(PACKAGE_ORG_JABREF_MODEL) - .layer("Cli").definedBy(PACKAGE_ORG_JABREF_CLI) - .layer("Migrations").definedBy("org.jabref.migrations..") // TODO: Move to logic - .layer("Preferences").definedBy("org.jabref.preferences..") - .layer("Styletester").definedBy("org.jabref.styletester..") - - .whereLayer("Gui").mayOnlyBeAccessedByLayers("Preferences", "Cli") // TODO: Remove preferences here - .whereLayer("Logic").mayOnlyBeAccessedByLayers("Gui", "Cli", "Model", "Migrations", "Preferences") - .whereLayer("Model").mayOnlyBeAccessedByLayers("Gui", "Logic", "Migrations", "Cli", "Preferences") - .whereLayer("Cli").mayNotBeAccessedByAnyLayer() - .whereLayer("Migrations").mayOnlyBeAccessedByLayers("Logic") - .whereLayer("Preferences").mayOnlyBeAccessedByLayers("Gui", "Logic", "Migrations", "Styletester", "Cli") // TODO: Remove logic here - - .check(classes); + .layer("Gui").definedBy(PACKAGE_ORG_JABREF_GUI) + .layer("Logic").definedBy(PACKAGE_ORG_JABREF_LOGIC) + .layer("Model").definedBy(PACKAGE_ORG_JABREF_MODEL) + .layer("Cli").definedBy(PACKAGE_ORG_JABREF_CLI) + .layer("Migrations").definedBy("org.jabref.migrations..") // TODO: Move to logic + .layer("Preferences").definedBy("org.jabref.preferences..") + .layer("Styletester").definedBy("org.jabref.styletester..") + + .whereLayer("Gui").mayOnlyBeAccessedByLayers("Preferences", "Cli") // TODO: Remove preferences here + .whereLayer("Logic").mayOnlyBeAccessedByLayers("Gui", "Cli", "Model", "Migrations", "Preferences") + .whereLayer("Model").mayOnlyBeAccessedByLayers("Gui", "Logic", "Migrations", "Cli", "Preferences") + .whereLayer("Cli").mayNotBeAccessedByAnyLayer() + .whereLayer("Migrations").mayOnlyBeAccessedByLayers("Logic") + .whereLayer("Preferences").mayOnlyBeAccessedByLayers("Gui", "Logic", "Migrations", "Styletester", "Cli") // TODO: Remove logic here + + .check(classes); } @ArchTest From 0422f521c77b868fbe78e69f1a47d6d96c9551b1 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 20:16:04 +0200 Subject: [PATCH 13/19] Add missing class --- .../org/jabref/architecture/AllowedToUseSwing.java | 10 ++++++++++ .../java/org/jabref/gui/desktop/JabRefDesktop.java | 1 + 2 files changed, 11 insertions(+) create mode 100644 src/main/java/org/jabref/architecture/AllowedToUseSwing.java diff --git a/src/main/java/org/jabref/architecture/AllowedToUseSwing.java b/src/main/java/org/jabref/architecture/AllowedToUseSwing.java new file mode 100644 index 00000000000..7c3cd0f9993 --- /dev/null +++ b/src/main/java/org/jabref/architecture/AllowedToUseSwing.java @@ -0,0 +1,10 @@ +package org.jabref.architecture; + +/** + * Annotation to indicate that this logic class can access AWT + */ +public @interface AllowedToUseSwing { + + // The rationale + String value(); +} diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java index d32a756430a..b74fea558d4 100644 --- a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java +++ b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java @@ -294,6 +294,7 @@ 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() { From 279ae2e25fe45a05b34c6d01fb4907c0e0dcdc81 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 20:39:26 +0200 Subject: [PATCH 14/19] remove check for empty file dirs --- src/main/java/org/jabref/model/database/BibDatabaseContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 299716792e6..03f04301b0c 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -145,7 +145,7 @@ public List getFileDirectories(FilePreferences preferences) { .ifPresent(metaDataDirectory -> fileDirs.add(getFileDirectoryPath(metaDataDirectory))); // 3. BIB file directory or Main file directory - if (fileDirs.isEmpty() && preferences.shouldStoreFilesRelativeToBibFile()) { + if (preferences.shouldStoreFilesRelativeToBibFile()) { getDatabasePath().ifPresent(dbPath -> { Path parentPath = dbPath.getParent(); if (parentPath == null) { From 2c0e172986f9304c8a7b20d638af14e33fb23b80 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 21:06:09 +0200 Subject: [PATCH 15/19] fix tests --- .../database/BibDatabaseContextTest.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 800af3896f0..da6562a65ba 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -3,7 +3,9 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; +import java.util.Optional; +import org.jabref.gui.desktop.JabRefDesktop; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.types.IEEETranEntryType; import org.jabref.model.metadata.MetaData; @@ -70,7 +72,9 @@ void getFileDirectoriesWithRelativeMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("../Literature"); - assertEquals(List.of(Path.of("/absolute/Literature").toAbsolutePath()), + // first directory is the metadata + // second directory is bib file location + assertEquals(List.of(Path.of("/absolute/Literature"), Path.of("/absolute/subdir")), database.getFileDirectories(fileDirPrefs)); } @@ -81,10 +85,26 @@ void getFileDirectoriesWithMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("Literature"); - assertEquals(List.of(Path.of("/absolute/subdir/Literature").toAbsolutePath()), + // first directory is the metadata + // second directory is bib file location + assertEquals(List.of(Path.of("/absolute/subdir/Literature"), Path.of("/absolute/subdir")), database.getFileDirectories(fileDirPrefs)); } + + @Test + void getUserFileDirectoryIfAllAreEmpty() { + when(fileDirPrefs.shouldStoreFilesRelativeToBibFile()).thenReturn(false); + + Path userDirJabRef = Path.of(JabRefDesktop.getDefaultFileChooserDirectory(), "JabRef"); + + when(fileDirPrefs.getFileDirectory()).thenReturn(Optional.of(userDirJabRef)); + BibDatabaseContext database = new BibDatabaseContext(); + database.setDatabasePath(Path.of("biblio.bib")); + assertEquals(Collections.singletonList(userDirJabRef), database.getFileDirectories(fileDirPrefs)); + } + + @Test void testTypeBasedOnDefaultBiblatex() { BibDatabaseContext bibDatabaseContext = new BibDatabaseContext(new BibDatabase(), new MetaData()); From 954d2760a5970d57f591821a091e02075193ecf9 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 21:09:01 +0200 Subject: [PATCH 16/19] Fix checkstye --- .../java/org/jabref/model/database/BibDatabaseContextTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index da6562a65ba..7c8e3f94eee 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -91,7 +91,6 @@ void getFileDirectoriesWithMetadata() { database.getFileDirectories(fileDirPrefs)); } - @Test void getUserFileDirectoryIfAllAreEmpty() { when(fileDirPrefs.shouldStoreFilesRelativeToBibFile()).thenReturn(false); @@ -104,7 +103,6 @@ void getUserFileDirectoryIfAllAreEmpty() { assertEquals(Collections.singletonList(userDirJabRef), database.getFileDirectories(fileDirPrefs)); } - @Test void testTypeBasedOnDefaultBiblatex() { BibDatabaseContext bibDatabaseContext = new BibDatabaseContext(new BibDatabase(), new MetaData()); From 21819e8e1a09b8329a715fa2d4fc11600353b651 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sat, 3 Sep 2022 21:13:29 +0200 Subject: [PATCH 17/19] fix copy paste error --- src/main/java/org/jabref/architecture/AllowedToUseSwing.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/architecture/AllowedToUseSwing.java b/src/main/java/org/jabref/architecture/AllowedToUseSwing.java index 7c3cd0f9993..99a40a4bade 100644 --- a/src/main/java/org/jabref/architecture/AllowedToUseSwing.java +++ b/src/main/java/org/jabref/architecture/AllowedToUseSwing.java @@ -1,7 +1,7 @@ package org.jabref.architecture; /** - * Annotation to indicate that this logic class can access AWT + * Annotation to indicate that this logic class can access swing */ public @interface AllowedToUseSwing { From 47ac5611736c741ea9f061568380bdca832581e0 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 21:24:11 +0200 Subject: [PATCH 18/19] Revert "remove check for empty file dirs" This reverts commit 279ae2e25fe45a05b34c6d01fb4907c0e0dcdc81. --- src/main/java/org/jabref/model/database/BibDatabaseContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 03f04301b0c..299716792e6 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -145,7 +145,7 @@ public List getFileDirectories(FilePreferences preferences) { .ifPresent(metaDataDirectory -> fileDirs.add(getFileDirectoryPath(metaDataDirectory))); // 3. BIB file directory or Main file directory - if (preferences.shouldStoreFilesRelativeToBibFile()) { + if (fileDirs.isEmpty() && preferences.shouldStoreFilesRelativeToBibFile()) { getDatabasePath().ifPresent(dbPath -> { Path parentPath = dbPath.getParent(); if (parentPath == null) { From da9d81adbb65dc05afd814289c9295af58f390e1 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 3 Sep 2022 21:50:03 +0200 Subject: [PATCH 19/19] Fix tests Co-authored-by: Christoph --- .../logic/cleanup/CleanupWorkerTest.java | 23 ++++++++++--------- .../database/BibDatabaseContextTest.java | 8 +++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java b/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java index 77e20ec00d1..6716d83976b 100644 --- a/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java +++ b/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java @@ -1,6 +1,5 @@ package org.jabref.logic.cleanup; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -48,19 +47,21 @@ class CleanupWorkerTest { private final CleanupPreset emptyPreset = new CleanupPreset(EnumSet.noneOf(CleanupPreset.CleanupStep.class)); private CleanupWorker worker; - // Ensure that the folder stays the same for all tests. By default @TempDir creates a new folder for each usage + // Ensure that the folder stays the same for all tests. + // By default, @TempDir creates a new folder for each usage private Path bibFolder; + // Currently, this directory is created below the bibFolder + private Path pdfPath; + @BeforeEach void setUp(@TempDir Path bibFolder) throws IOException { - this.bibFolder = bibFolder; - Path path = bibFolder.resolve("ARandomlyNamedFolder"); - Files.createDirectory(path); - File pdfFolder = path.toFile(); + pdfPath = bibFolder.resolve("ARandomlyNamedFolder"); + Files.createDirectory(pdfPath); MetaData metaData = new MetaData(); - metaData.setDefaultFileDirectory(pdfFolder.getAbsolutePath()); + metaData.setDefaultFileDirectory(pdfPath.toAbsolutePath().toString()); BibDatabaseContext context = new BibDatabaseContext(new BibDatabase(), metaData); Files.createFile(bibFolder.resolve("test.bib")); context.setDatabasePath(bibFolder.resolve("test.bib")); @@ -239,7 +240,7 @@ void cleanupMoveFilesMovesFileFromSubfolder(@TempDir Path bibFolder) throws IOEx void cleanupRelativePathsConvertAbsoluteToRelativePath() throws IOException { CleanupPreset preset = new CleanupPreset(CleanupPreset.CleanupStep.MAKE_PATHS_RELATIVE); - Path path = bibFolder.resolve("AnotherRandomlyNamedFile"); + Path path = pdfPath.resolve("AnotherRandomlyNamedFile"); Files.createFile(path); BibEntry entry = new BibEntry(); LinkedFile fileField = new LinkedFile("", path.toAbsolutePath(), ""); @@ -254,10 +255,10 @@ void cleanupRelativePathsConvertAbsoluteToRelativePath() throws IOException { void cleanupRenamePdfRenamesRelativeFile() throws IOException { CleanupPreset preset = new CleanupPreset(CleanupPreset.CleanupStep.RENAME_PDF); - Path path = bibFolder.resolve("AnotherRandomlyNamedFile.tmp"); + Path path = pdfPath.resolve("AnotherRandomlyNamedFile.tmp"); Files.createFile(path); - BibEntry entry = new BibEntry(); - entry.setCitationKey("Toot"); + BibEntry entry = new BibEntry() + .withCitationKey("Toot"); LinkedFile fileField = new LinkedFile("", path.toAbsolutePath(), ""); entry.setField(StandardField.FILE, FileFieldWriter.getStringRepresentation(fileField)); diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 7c8e3f94eee..181117f7dac 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -73,8 +73,8 @@ void getFileDirectoriesWithRelativeMetadata() { database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("../Literature"); // first directory is the metadata - // second directory is bib file location - assertEquals(List.of(Path.of("/absolute/Literature"), Path.of("/absolute/subdir")), + // the bib file location is not included, because only the library-configured directories should be searched and the fallback should be the global directory. + assertEquals(List.of(Path.of("/absolute/Literature").toAbsolutePath()), database.getFileDirectories(fileDirPrefs)); } @@ -86,8 +86,8 @@ void getFileDirectoriesWithMetadata() { database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("Literature"); // first directory is the metadata - // second directory is bib file location - assertEquals(List.of(Path.of("/absolute/subdir/Literature"), Path.of("/absolute/subdir")), + // the bib file location is not included, because only the library-configured directories should be searched and the fallback should be the global directory. + assertEquals(List.of(Path.of("/absolute/subdir/Literature").toAbsolutePath()), database.getFileDirectories(fileDirPrefs)); }