From 20d8bab94f1b37b95d83bcf1c4bf33a4da8ca306 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 15 Jul 2024 02:17:23 +0200 Subject: [PATCH 1/5] Fix handling of relative-file storage and auto linking --- CHANGELOG.md | 2 ++ .../gui/externalfiles/AutoSetFileLinksUtil.java | 7 +++++-- .../gui/fieldeditors/LinkedFilesEditorViewModel.java | 9 ++++++++- .../logic/util/io/CitationKeyBasedFileFinder.java | 10 +++++++++- .../java/org/jabref/logic/util/io/FileFinders.java | 2 +- .../jabref/model/database/BibDatabaseContext.java | 12 ++++++------ 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1574426e4ca..8f29e803c00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Changed +- JabRef respects the [configuration for storing files relative to the .bib file](https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in more cases. + ### Fixed ### Removed diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java index bd3f494e3c6..7733bedfb07 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java @@ -6,7 +6,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import org.jabref.gui.externalfiletype.ExternalFileType; import org.jabref.gui.externalfiletype.ExternalFileTypes; @@ -52,6 +51,7 @@ public List getFileExceptions() { } private static final Logger LOGGER = LoggerFactory.getLogger(AutoSetFileLinksUtil.class); + private final List directories; private final AutoLinkPreferences autoLinkPreferences; private final FilePreferences filePreferences; @@ -106,7 +106,9 @@ public LinkFilesResult linkAssociatedFiles(List entries, NamedCompound public List findAssociatedNotLinkedFiles(BibEntry entry) throws IOException { List linkedFiles = new ArrayList<>(); - List extensions = filePreferences.getExternalFileTypes().stream().map(ExternalFileType::getExtension).collect(Collectors.toList()); + List extensions = filePreferences.getExternalFileTypes().stream().map(ExternalFileType::getExtension).toList(); + + LOGGER.debug("Searching for extensions {} in directories {}", extensions, directories); // Run the search operation FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPreferences); @@ -134,6 +136,7 @@ public List findAssociatedNotLinkedFiles(BibEntry entry) throws IOEx Path relativeFilePath = FileUtil.relativize(foundFile, directories); LinkedFile linkedFile = new LinkedFile("", relativeFilePath, strType); linkedFiles.add(linkedFile); + LOGGER.debug("Found file {} for entry {}", linkedFile, entry.getCitationKey()); } } diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index 005ef6e75ff..f628f9514bf 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -190,9 +190,15 @@ public void bindToEntry(BibEntry entry) { super.bindToEntry(entry); if ((entry != null) && preferences.getEntryEditorPreferences().autoLinkFilesEnabled()) { + LOGGER.debug("Auto-linking files for entry {}", entry); BackgroundTask> findAssociatedNotLinkedFiles = BackgroundTask .wrap(() -> findAssociatedNotLinkedFiles(entry)) - .onSuccess(files::addAll); + .onSuccess(list -> { + if (!list.isEmpty()) { + LOGGER.debug("Found non-associated files:", list); + files.addAll(list); + } + }); taskExecutor.execute(findAssociatedNotLinkedFiles); } } @@ -224,6 +230,7 @@ private List findAssociatedNotLinkedFiles(BibEntry entry) { dialogService.showErrorDialogAndWait("Error accessing the file system", e); } + LOGGER.trace("Found {} associated files for entry {}", result.size(), entry.getCitationKey()); return result; } diff --git a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java index 3e8064eaaed..b6fd974e3b8 100644 --- a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java @@ -21,8 +21,13 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.strings.StringUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + class CitationKeyBasedFileFinder implements FileFinder { + private static final Logger LOGGER = LoggerFactory.getLogger(CitationKeyBasedFileFinder.class); + private final boolean exactKeyOnly; CitationKeyBasedFileFinder(boolean exactKeyOnly) { @@ -36,6 +41,7 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li Optional citeKeyOptional = entry.getCitationKey(); if (StringUtil.isBlank(citeKeyOptional)) { + LOGGER.debug("No citation key found in entry {}", entry); return Collections.emptyList(); } String citeKey = citeKeyOptional.get(); @@ -52,16 +58,18 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li // First, look for exact matches if (nameWithoutExtension.equals(citeKey)) { + LOGGER.debug("Found exact match for key {} in file {}", citeKey, file); result.add(file); continue; } // If we get here, we did not find any exact matches. If non-exact matches are allowed, try to find one if (!exactKeyOnly && matches(name, citeKey)) { + LOGGER.debug("Found non-exact match for key {} in file {}", citeKey, file); result.add(file); } } - return result.stream().sorted().collect(Collectors.toList()); + return result.stream().sorted().toList(); } private boolean matches(String filename, String citeKey) { diff --git a/src/main/java/org/jabref/logic/util/io/FileFinders.java b/src/main/java/org/jabref/logic/util/io/FileFinders.java index de46e3dbbd0..ca27713a3fa 100644 --- a/src/main/java/org/jabref/logic/util/io/FileFinders.java +++ b/src/main/java/org/jabref/logic/util/io/FileFinders.java @@ -2,7 +2,7 @@ public class FileFinders { /** - * Creates a preconfigurated file finder based on the given AutoLink preferences. + * Creates a preconfigured file finder based on the given AutoLink preferences. */ public static FileFinder constructFromConfiguration(AutoLinkPreferences autoLinkPreferences) { return switch (autoLinkPreferences.getCitationKeyDependency()) { diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 6e523fa29e3..17187b2c199 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -7,7 +7,6 @@ import java.util.Objects; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; import org.jabref.architecture.AllowedToUseLogic; import org.jabref.gui.LibraryTab; @@ -170,10 +169,12 @@ public List getFileDirectories(FilePreferences preferences) { metaData.getDefaultFileDirectory() .ifPresent(metaDataDirectory -> fileDirs.add(getFileDirectoryPath(metaDataDirectory))); + // fileDirs.isEmpty() is true if: + // 1) no user-specific file directory set (in the metadata of the bib file) and + // 2) no general file directory is set (in the metadata of the bib file) + // 3. BIB file directory or main file directory - // fileDirs.isEmpty in the case, 1) no user-specific file directory and 2) no general file directory is set - // (in the metadata of the bib file) - if (fileDirs.isEmpty() && preferences.shouldStoreFilesRelativeToBibFile()) { + if (preferences.shouldStoreFilesRelativeToBibFile()) { getDatabasePath().ifPresent(dbPath -> { Path parentPath = dbPath.getParent(); if (parentPath == null) { @@ -183,11 +184,10 @@ public List getFileDirectories(FilePreferences preferences) { fileDirs.add(parentPath); }); } else { - // Main file directory preferences.getMainFileDirectory().ifPresent(fileDirs::add); } - return fileDirs.stream().map(Path::toAbsolutePath).collect(Collectors.toList()); + return fileDirs.stream().map(Path::toAbsolutePath).distinct().toList(); } /** From 6e7051ba78cb25369fbe2f8ce55cf2a759749eeb Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 15 Jul 2024 02:19:41 +0200 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f29e803c00..1bb4a8133ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Changed -- JabRef respects the [configuration for storing files relative to the .bib file](https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in more cases. +- JabRef respects the [configuration for storing files relative to the .bib file](https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in more cases. [#11492](https://github.com/JabRef/jabref/pull/11492) ### Fixed From de720235e5b986cde3230204b7a977f00af8e08b Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 15 Jul 2024 02:33:40 +0200 Subject: [PATCH 3/5] Adapt tests to new behavior --- .../model/database/BibDatabaseContextTest.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 26777d80519..86f29a486d2 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -76,8 +76,10 @@ void getFileDirectoriesWithRelativeMetadata() { database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("../Literature"); // first directory is the metadata - // 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()), + assertEquals(List.of( + Path.of("/absolute/Literature").toAbsolutePath(), + Path.of("/absolute/subdir").toAbsolutePath() + ), database.getFileDirectories(fileDirPrefs)); } @@ -88,9 +90,11 @@ void getFileDirectoriesWithMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("Literature"); - // first directory is the metadata - // 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()), + assertEquals(List.of( + // first directory is the metadata + Path.of("/absolute/subdir/Literature").toAbsolutePath(), + Path.of("/absolute/subdir").toAbsolutePath() + ), database.getFileDirectories(fileDirPrefs)); } From 8dc30331ebe5a68d6a2ca796732f29f7c1a4b71f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 15 Jul 2024 09:18:52 +0200 Subject: [PATCH 4/5] Refine code --- .../model/database/BibDatabaseContext.java | 50 +++++++++++-------- .../database/BibDatabaseContextTest.java | 24 ++++++--- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 17187b2c199..65a02a838a1 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -153,41 +153,45 @@ public boolean isStudy() { *
  • user-specific metadata directory
    • *
  • general metadata directory
    • *
  • BIB file directory (if configured in the preferences AND none of the two above directories are configured)
    • - *
  • preferences directory (if .bib file directory should not be used according to the preferences)
    • + *
  • preferences directory (if .bib file directory should not be used according to the (global) preferences)
    • * * * @param preferences The fileDirectory preferences */ public List getFileDirectories(FilePreferences preferences) { - List fileDirs = new ArrayList<>(); + List fileDirs = new ArrayList<>(3); - // 1. Metadata user-specific directory - metaData.getUserFileDirectory(preferences.getUserAndHost()) - .ifPresent(userFileDirectory -> fileDirs.add(getFileDirectoryPath(userFileDirectory))); + Optional userFileDirectory = metaData.getUserFileDirectory(preferences.getUserAndHost()).map(dir -> getFileDirectoryPath(dir)); + userFileDirectory.ifPresent(fileDirs::add); - // 2. Metadata general directory - metaData.getDefaultFileDirectory() - .ifPresent(metaDataDirectory -> fileDirs.add(getFileDirectoryPath(metaDataDirectory))); + Optional generalFileDirectory = metaData.getDefaultFileDirectory().map(dir -> getFileDirectoryPath(dir)); + generalFileDirectory.ifPresent(fileDirs::add); - // fileDirs.isEmpty() is true if: + // fileDirs.isEmpty() is true after these two if there are no directories set in the BIB file itself: // 1) no user-specific file directory set (in the metadata of the bib file) and // 2) no general file directory is set (in the metadata of the bib file) - // 3. BIB file directory or main file directory + // BIB file directory or main file directory (according to (global) preferences) if (preferences.shouldStoreFilesRelativeToBibFile()) { getDatabasePath().ifPresent(dbPath -> { Path parentPath = dbPath.getParent(); if (parentPath == null) { parentPath = Path.of(System.getProperty("user.dir")); + LOGGER.warn("Parent path of database file {} is null. Falling back to {}.", dbPath, parentPath); } Objects.requireNonNull(parentPath, "BibTeX database parent path is null"); - fileDirs.add(parentPath); + Path absolutePath = parentPath.toAbsolutePath(); + if (!fileDirs.contains(absolutePath)) { + fileDirs.add(absolutePath); + } }); } else { - preferences.getMainFileDirectory().ifPresent(fileDirs::add); + preferences.getMainFileDirectory() + .filter(path -> !fileDirs.contains(path)) + .ifPresent(fileDirs::add); } - return fileDirs.stream().map(Path::toAbsolutePath).distinct().toList(); + return fileDirs; } /** @@ -201,15 +205,19 @@ public Optional getFirstExistingFileDir(FilePreferences preferences) { .findFirst(); } - private Path getFileDirectoryPath(String directoryName) { - Path directory = Path.of(directoryName); - // If this directory is relative, we try to interpret it as relative to - // the file path of this BIB file: - Optional databaseFile = getDatabasePath(); - if (!directory.isAbsolute() && databaseFile.isPresent()) { - return databaseFile.get().getParent().resolve(directory).normalize(); + /** + * @return The absolute path for the given directory + */ + private Path getFileDirectoryPath(String directory) { + Path path = Path.of(directory); + if (path.isAbsolute()) { + return path; } - return directory; + + // If this path is relative, we try to interpret it as relative to the file path of this BIB file: + return getDatabasePath() + .map(databaseFile -> databaseFile.getParent().resolve(path).normalize().toAbsolutePath()) + .orElse(path); } public DatabaseSynchronizer getDBMSSynchronizer() { diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 86f29a486d2..79abcd8994d 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java @@ -1,7 +1,6 @@ package org.jabref.model.database; import java.nio.file.Path; -import java.util.Collections; import java.util.List; import java.util.Optional; @@ -38,7 +37,7 @@ void setUp() { void getFileDirectoriesWithEmptyDbParent() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(Path.of("biblio.bib")); - assertEquals(Collections.singletonList(currentWorkingDir), database.getFileDirectories(fileDirPrefs)); + assertEquals(List.of(currentWorkingDir), database.getFileDirectories(fileDirPrefs)); } @Test @@ -47,7 +46,7 @@ void getFileDirectoriesWithRelativeDbParent() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); - assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); + assertEquals(List.of(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); } @Test @@ -56,7 +55,16 @@ void getFileDirectoriesWithRelativeDottedDbParent() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); - assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); + assertEquals(List.of(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); + } + + @Test + void getFileDirectoriesWithDotAsDirectory() { + Path file = Path.of("biblio.bib"); + BibDatabaseContext database = new BibDatabaseContext(); + database.setDatabasePath(currentWorkingDir.resolve(file)); + database.getMetaData().setDefaultFileDirectory("."); + assertEquals(List.of(currentWorkingDir), database.getFileDirectories(fileDirPrefs)); } @Test @@ -65,7 +73,7 @@ void getFileDirectoriesWithAbsoluteDbParent() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); - assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); + assertEquals(List.of(currentWorkingDir.resolve(file.getParent())), database.getFileDirectories(fileDirPrefs)); } @Test @@ -75,8 +83,8 @@ void getFileDirectoriesWithRelativeMetadata() { BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("../Literature"); - // first directory is the metadata assertEquals(List.of( + // first directory originates from the metadata Path.of("/absolute/Literature").toAbsolutePath(), Path.of("/absolute/subdir").toAbsolutePath() ), @@ -91,7 +99,7 @@ void getFileDirectoriesWithMetadata() { database.setDatabasePath(file); database.getMetaData().setDefaultFileDirectory("Literature"); assertEquals(List.of( - // first directory is the metadata + // first directory originates from the metadata Path.of("/absolute/subdir/Literature").toAbsolutePath(), Path.of("/absolute/subdir").toAbsolutePath() ), @@ -106,7 +114,7 @@ void getUserFileDirectoryIfAllAreEmpty() { when(fileDirPrefs.getMainFileDirectory()).thenReturn(Optional.of(userDirJabRef)); BibDatabaseContext database = new BibDatabaseContext(); database.setDatabasePath(Path.of("biblio.bib")); - assertEquals(Collections.singletonList(userDirJabRef), database.getFileDirectories(fileDirPrefs)); + assertEquals(List.of(userDirJabRef), database.getFileDirectories(fileDirPrefs)); } @Test From a9a8ab19f5d3ac534be6d677e0c166d27abd791a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Tue, 16 Jul 2024 09:34:59 +0200 Subject: [PATCH 5/5] Use LinkedHashSet instead of List for building the resulting list --- .../org/jabref/model/database/BibDatabaseContext.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 65a02a838a1..8743c243749 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -3,6 +3,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -159,7 +160,8 @@ public boolean isStudy() { * @param preferences The fileDirectory preferences */ public List getFileDirectories(FilePreferences preferences) { - List fileDirs = new ArrayList<>(3); + // Paths are a) ordered and b) should be contained only once in the result + LinkedHashSet fileDirs = new LinkedHashSet<>(3); Optional userFileDirectory = metaData.getUserFileDirectory(preferences.getUserAndHost()).map(dir -> getFileDirectoryPath(dir)); userFileDirectory.ifPresent(fileDirs::add); @@ -180,10 +182,7 @@ public List getFileDirectories(FilePreferences preferences) { LOGGER.warn("Parent path of database file {} is null. Falling back to {}.", dbPath, parentPath); } Objects.requireNonNull(parentPath, "BibTeX database parent path is null"); - Path absolutePath = parentPath.toAbsolutePath(); - if (!fileDirs.contains(absolutePath)) { - fileDirs.add(absolutePath); - } + fileDirs.add(parentPath.toAbsolutePath()); }); } else { preferences.getMainFileDirectory() @@ -191,7 +190,7 @@ public List getFileDirectories(FilePreferences preferences) { .ifPresent(fileDirs::add); } - return fileDirs; + return new ArrayList<>(fileDirs); } /**