Fix handling of relative-file storage and auto linking (#11492)

* Fix handling of relative-file storage and auto linking * Update CHANGELOG.md * Adapt tests to new behavior * Refine code * Use LinkedHashSet instead of List for building the resulting list
JabRef · Jul 16, 2024 · a5e1bd6 · a5e1bd6
1 parent 0fef230
commit a5e1bd6
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,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. [#11492](https://github.com/JabRef/jabref/pull/11492)
+
 ### Fixed
 
 ### Removed

diff --git 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<IOException> getFileExceptions() {
     }
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AutoSetFileLinksUtil.class);
+
     private final List<Path> directories;
     private final AutoLinkPreferences autoLinkPreferences;
     private final FilePreferences filePreferences;
@@ -106,7 +106,9 @@ public LinkFilesResult linkAssociatedFiles(List<BibEntry> entries, NamedCompound
     public List<LinkedFile> findAssociatedNotLinkedFiles(BibEntry entry) throws IOException {
         List<LinkedFile> linkedFiles = new ArrayList<>();
 
-        List<String> extensions = filePreferences.getExternalFileTypes().stream().map(ExternalFileType::getExtension).collect(Collectors.toList());
+        List<String> 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<LinkedFile> 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
@@ -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<List<LinkedFileViewModel>> 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<LinkedFileViewModel> 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
@@ -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<Path> findAssociatedFiles(BibEntry entry, List<Path> directories, Li
 
         Optional<String> 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<Path> findAssociatedFiles(BibEntry entry, List<Path> 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
@@ -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
@@ -3,11 +3,11 @@
 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;
 import java.util.UUID;
-import java.util.stream.Collectors;
 
 import org.jabref.architecture.AllowedToUseLogic;
 import org.jabref.gui.LibraryTab;
@@ -154,40 +154,43 @@ public boolean isStudy() {
      *     <li>user-specific metadata directory</li>
      *     <li>general metadata directory</li>
      *     <li>BIB file directory (if configured in the preferences AND none of the two above directories are configured)</li>
-     *     <li>preferences directory (if .bib file directory should not be used according to the preferences)</li>
+     *     <li>preferences directory (if .bib file directory should not be used according to the (global) preferences)</li>
      * </ol>
      *
      * @param preferences The fileDirectory preferences
      */
     public List<Path> getFileDirectories(FilePreferences preferences) {
-        List<Path> fileDirs = new ArrayList<>();
+        // Paths are a) ordered and b) should be contained only once in the result
+        LinkedHashSet<Path> fileDirs = new LinkedHashSet<>(3);
 
-        // 1. Metadata user-specific directory
-        metaData.getUserFileDirectory(preferences.getUserAndHost())
-                .ifPresent(userFileDirectory -> fileDirs.add(getFileDirectoryPath(userFileDirectory)));
+        Optional<Path> 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<Path> generalFileDirectory = metaData.getDefaultFileDirectory().map(dir -> getFileDirectoryPath(dir));
+        generalFileDirectory.ifPresent(fileDirs::add);
 
-        // 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()) {
+        // 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)
+
+        // 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);
+                fileDirs.add(parentPath.toAbsolutePath());
             });
         } else {
-            // Main file directory
-            preferences.getMainFileDirectory().ifPresent(fileDirs::add);
+            preferences.getMainFileDirectory()
+                       .filter(path -> !fileDirs.contains(path))
+                       .ifPresent(fileDirs::add);
         }
 
-        return fileDirs.stream().map(Path::toAbsolutePath).collect(Collectors.toList());
+        return new ArrayList<>(fileDirs);
     }
 
     /**
@@ -201,15 +204,19 @@ public Optional<Path> 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<Path> 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
@@ -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,9 +83,11 @@ void getFileDirectoriesWithRelativeMetadata() {
         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/Literature").toAbsolutePath()),
+        assertEquals(List.of(
+                        // first directory originates from the metadata
+                        Path.of("/absolute/Literature").toAbsolutePath(),
+                        Path.of("/absolute/subdir").toAbsolutePath()
+                ),
                 database.getFileDirectories(fileDirPrefs));
     }
 
@@ -88,9 +98,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 originates from the metadata
+                        Path.of("/absolute/subdir/Literature").toAbsolutePath(),
+                        Path.of("/absolute/subdir").toAbsolutePath()
+                ),
                 database.getFileDirectories(fileDirPrefs));
     }
 
@@ -102,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