Fixes #3472: Files starting with BibTeX key of a similar entry are no…

… longer found by mistake

CHANGELOG.md

@@ -51,6 +51,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
  - We fixed an issue where the same Java Look and Feel would be listed more than once in the Preferences. [#3391](https://github.com/JabRef/jabref/issues/3391)
  - We fixed an issue where errors in citation styles triggered an exception when opening the preferences dialog [#3389](https://github.com/JabRef/jabref/issues/3389)
  - We fixed an issue where entries were displayed twice after insertion into a shared database. [#3164](https://github.com/JabRef/jabref/issues/3164)
+ - We improved the auto link algorithm so that files starting with a similar key are no longer found (e.g, `Einstein1902` vs `Einstein1902a`). [#3472](https://github.com/JabRef/jabref/issues/3472) 
  - We fixed an issue where special fields (such as `printed`) could not be cleared when syncing special fields via the keywords [#3432](https://github.com/JabRef/jabref/issues/3432)
  - We fixed an issue where the tooltip of the global search bar showed html tags instead of formatting the text [#3381](https://github.com/JabRef/jabref/issues/3381)
  - We fixed an issue where timestamps were not updated for changed entries [#2810](https://github.com/JabRef/jabref/issues/2810)

src/main/java/org/jabref/gui/BasePanel.java

@@ -2016,9 +2016,9 @@ public void searchAndOpen() {
 
             // Run the search operation:
             FileFinder fileFinder = FileFinders.constructFromConfiguration(Globals.prefs.getAutoLinkPreferences());
-            List<Path> files = fileFinder.findAssociatedFiles(entry, dirs, extensions);
+            Collection<Path> files = fileFinder.findAssociatedFiles(entry, dirs, extensions);
             if (!files.isEmpty()) {
-                Path file = files.get(0);
+                Path file = files.iterator().next();
                 Optional<ExternalFileType> type = ExternalFileTypes.getInstance().getExternalFileTypeByFile(file);
                 if (type.isPresent()) {
                     try {

src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java

@@ -4,6 +4,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -47,7 +48,7 @@ public List<LinkedFile> findAssociatedNotLinkedFiles(BibEntry entry) {
 
         // Run the search operation
         FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPreferences);
-        List<Path> result = fileFinder.findAssociatedFiles(entry, directories, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(entry, directories, extensions);
 
         // Collect the found files that are not yet linked
         for (Path foundFile : result) {

src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java

@@ -22,7 +22,7 @@ public class BibtexKeyPatternUtil extends BracketedPattern {
     private static final Log LOGGER = LogFactory.getLog(BibtexKeyPatternUtil.class);
 
     // All single characters that we can use for extending a key to make it unique:
-    private static final String CHARS = "abcdefghijklmnopqrstuvwxyz";
+    public static final String CHARS = "abcdefghijklmnopqrstuvwxyz";
 
     private BibtexKeyPatternUtil() {
     }

src/main/java/org/jabref/logic/net/URLDownload.java

@@ -259,7 +259,7 @@ public Path toTemporaryFile() throws IOException {
 
         // Take everything after the last '/' as name + extension
         String fileNameWithExtension = sourcePath.substring(sourcePath.lastIndexOf('/') + 1);
-        String fileName = FileUtil.getFileName(fileNameWithExtension);
+        String fileName = FileUtil.getBaseName(fileNameWithExtension);
         String extension = "." + FileHelper.getFileExtension(fileNameWithExtension).orElse("tmp");
 
         // Create temporary file and download to it

src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java

@@ -13,7 +13,9 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.jabref.logic.bibtexkeypattern.BibtexKeyPatternUtil;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.strings.StringUtil;
 import org.jabref.model.util.FileHelper;
 
 import com.google.common.collect.ArrayListMultimap;
@@ -44,22 +46,21 @@ public Multimap<BibEntry, Path> findAssociatedFiles(List<BibEntry> entries, List
         nextFile:
         for (Path file : filesWithExtension) {
             String name = file.getFileName().toString();
-            int dot = name.lastIndexOf('.');
-            // First, look for exact matches:
+            String nameWithoutExtension = FileUtil.getBaseName(name);
+
+            // First, look for exact matches
             for (BibEntry entry : entries) {
                 Optional<String> citeKey = entry.getCiteKeyOptional();
-                if ((citeKey.isPresent()) && !citeKey.get().isEmpty() && (dot > 0)
-                        && name.substring(0, dot).equals(citeKey.get())) {
+                if (StringUtil.isNotBlank(citeKey) && nameWithoutExtension.equals(citeKey.get())) {
                     result.get(entry).add(file);
                     continue nextFile;
                 }
             }
-            // If we get here, we did not find any exact matches. If non-exact
-            // matches are allowed, try to find one:
+            // If we get here, we did not find any exact matches. If non-exact matches are allowed, try to find one
             if (!exactKeyOnly) {
                 for (BibEntry entry : entries) {
                     Optional<String> citeKey = entry.getCiteKeyOptional();
-                    if ((citeKey.isPresent()) && !citeKey.get().isEmpty() && name.startsWith(citeKey.get())) {
+                    if (StringUtil.isNotBlank(citeKey) && matches(name, citeKey.get())) {
                         result.get(entry).add(file);
                         continue nextFile;
                     }
@@ -70,10 +71,21 @@ public Multimap<BibEntry, Path> findAssociatedFiles(List<BibEntry> entries, List
         return result;
     }
 
+    private boolean matches(String filename, String citeKey) {
+        boolean startsWithKey = filename.startsWith(citeKey);
+        if (startsWithKey) {
+            // The file name starts with the key, that's already a good start
+            // However, we do not want to match "JabRefa" for "JabRef" since this is probably a file belonging to another entry published in the same time / same name
+            char charAfterKey = filename.charAt(citeKey.length());
+            return !StringUtil.contains(BibtexKeyPatternUtil.CHARS, charAfterKey);
+        }
+        return false;
+    }
+
     /**
      * Returns a list of all files in the given directories which have one of the given extension.
      */
-    public Set<Path> findFilesByExtension(List<Path> directories, List<String> extensions) {
+    private Set<Path> findFilesByExtension(List<Path> directories, List<String> extensions) {
         Objects.requireNonNull(extensions, "Extensions must not be null!");
 
         BiPredicate<Path, BasicFileAttributes> isFileWithCorrectExtension = (path, attributes) ->

src/main/java/org/jabref/logic/util/io/FileUtil.java

@@ -27,6 +27,7 @@
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.util.OptionalUtil;
 
+import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -65,13 +66,8 @@ public static Optional<String> getFileExtension(File file) {
     /**
      * Returns the name part of a file name (i.e., everything in front of last ".").
      */
-    public static String getFileName(String fileNameWithExtension) {
-        int dotPosition = fileNameWithExtension.lastIndexOf('.');
-        if (dotPosition >= 0) {
-            return fileNameWithExtension.substring(0, dotPosition);
-        } else {
-            return fileNameWithExtension;
-        }
+    public static String getBaseName(String fileNameWithExtension) {
+        return FilenameUtils.getBaseName(fileNameWithExtension);
     }
 
     /**
@@ -80,7 +76,7 @@ public static String getFileName(String fileNameWithExtension) {
      * Currently, only the length is restricted to 255 chars, see MAXIMUM_FILE_NAME_LENGTH.
      */
     public static String getValidFileName(String fileName) {
-        String nameWithoutExtension = getFileName(fileName);
+        String nameWithoutExtension = getBaseName(fileName);
 
         if (nameWithoutExtension.length() > MAXIMUM_FILE_NAME_LENGTH) {
             Optional<String> extension = getFileExtension(fileName);

src/main/java/org/jabref/model/strings/StringUtil.java

@@ -719,4 +719,8 @@ public static List<String> getStringAsWords(String text) {
     public static boolean containsIgnoreCase(String text, String searchString) {
         return StringUtils.containsIgnoreCase(text, searchString);
     }
+
+    public static boolean contains(String string, char character) {
+        return string.indexOf(character) != -1;
+    }
 }

src/test/java/org/jabref/BibtexTestData.java

@@ -16,7 +16,7 @@ public static BibEntry getBibtexEntry(ImportFormatPreferences importFormatPrefer
         return database.getEntryByKey("HipKro03").get();
     }
 
-    public static BibDatabase getBibtexDatabase(ImportFormatPreferences importFormatPreferences) throws IOException {
+    private static BibDatabase getBibtexDatabase(ImportFormatPreferences importFormatPreferences) throws IOException {
         String article = "@ARTICLE{HipKro03,\n" + "  author = {Eric von Hippel and Georg von Krogh},\n"
                         + "  title = {Open Source Software and the \"Private-Collective\" Innovation Model: Issues for Organization Science},\n"
                         + "  journal = {Organization Science},\n" + "  year = {2003},\n" + "  volume = {14},\n"

src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java

@@ -3,29 +3,23 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 import java.util.stream.Collectors;
 
-import org.jabref.BibtexTestData;
-import org.jabref.logic.bibtex.FieldContentParserPreferences;
-import org.jabref.logic.importer.ImportFormatPreferences;
-import org.jabref.model.database.BibDatabase;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.BibtexEntryTypes;
 
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
 public class CiteKeyBasedFileFinderTest {
@@ -34,26 +28,26 @@ public class CiteKeyBasedFileFinderTest {
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
     private BibEntry entry;
     private Path rootDir;
-    @Mock
-    private ImportFormatPreferences prefs;
+    private Path graphicsDir;
+    private Path pdfsDir;
+    private Path jpgFile;
+    private Path pdfFile;
 
     @Before
     public void setUp() throws IOException {
-        when(prefs.getFieldContentParserPreferences()).thenReturn(new FieldContentParserPreferences());
-
-        BibDatabase database = BibtexTestData.getBibtexDatabase(prefs);
-        entry = database.getEntries().iterator().next();
+        entry = new BibEntry(BibtexEntryTypes.ARTICLE.getName());
+        entry.setCiteKey("HipKro03");
 
         rootDir = temporaryFolder.getRoot().toPath();
 
         Path subDir = Files.createDirectory(rootDir.resolve("Organization Science"));
-        Path pdfSubDir = Files.createDirectory(rootDir.resolve("pdfs"));
+        pdfsDir = Files.createDirectory(rootDir.resolve("pdfs"));
 
         Files.createFile(subDir.resolve("HipKro03 - Hello.pdf"));
         Files.createFile(rootDir.resolve("HipKro03 - Hello.pdf"));
 
-        Path pdfSubSubDir = Files.createDirectory(pdfSubDir.resolve("sub"));
-        Files.createFile(pdfSubSubDir.resolve("HipKro03-sub.pdf"));
+        Path pdfSubSubDir = Files.createDirectory(pdfsDir.resolve("sub"));
+        pdfFile = Files.createFile(pdfSubSubDir.resolve("HipKro03-sub.pdf"));
 
         Files.createDirectory(rootDir.resolve("2002"));
         Path dir2003 = Files.createDirectory(rootDir.resolve("2003"));
@@ -65,41 +59,53 @@ public void setUp() throws IOException {
         Files.createFile(dirTest.resolve("TE.ST"));
         Files.createFile(dirTest.resolve("foo.dat"));
 
-        Path graphicsDir = Files.createDirectory(rootDir.resolve("graphicsDir"));
+        graphicsDir = Files.createDirectory(rootDir.resolve("graphicsDir"));
         Path graphicsSubDir = Files.createDirectories(graphicsDir.resolve("subDir"));
 
-        Files.createFile(graphicsSubDir.resolve("HipKro03test.jpg"));
-        Files.createFile(graphicsSubDir.resolve("HipKro03test.png"));
-
+        jpgFile = Files.createFile(graphicsSubDir.resolve("HipKro03 test.jpg"));
+        Files.createFile(graphicsSubDir.resolve("HipKro03 test.png"));
     }
 
     @Test
-    public void testFindAssociatedFiles() {
+    public void findAssociatedFilesInSubDirectories() {
         List<String> extensions = Arrays.asList("jpg", "pdf");
-        List<Path> dirs = Arrays.asList(rootDir.resolve("graphicsDir"), rootDir.resolve("pdfs"));
+        List<Path> dirs = Arrays.asList(graphicsDir, pdfsDir);
         FileFinder fileFinder = new CiteKeyBasedFileFinder(false);
-        List<Path> results = fileFinder.findAssociatedFiles(entry, dirs, extensions);
 
-        Path jpgFile = rootDir.resolve(Paths.get("graphicsDir", "subDir", "HipKro03test.jpg"));
-        Path pdfFile = rootDir.resolve(Paths.get("pdfs", "sub", "HipKro03-sub.pdf"));
+        Collection<Path> results = fileFinder.findAssociatedFiles(entry, dirs, extensions);
 
         assertEquals(Arrays.asList(jpgFile, pdfFile), results.stream().sorted().collect(Collectors.toList()));
     }
 
     @Test
-    public void findFilesByExtensionInNonExistingDirectoryFindsNothing() {
+    public void findAssociatedFilesIgnoresFilesStartingWithKeyButContinueWithText() throws Exception {
+        Files.createFile(pdfsDir.resolve("HipKro03a - Hello second paper.pdf"));
+        FileFinder fileFinder = new CiteKeyBasedFileFinder(false);
+
+        Collection<Path> results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf"));
+
+        assertEquals(Collections.singletonList(pdfFile), results);
+    }
+
+    @Test
+    public void findAssociatedFilesFindsFilesStartingWithKey() throws Exception {
+        Path secondPdfFile = Files.createFile(pdfsDir.resolve("HipKro03_Hello second paper.pdf"));
+        FileFinder fileFinder = new CiteKeyBasedFileFinder(false);
+
+        Collection<Path> results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf"));
+
+        assertEquals(Arrays.asList(pdfFile, secondPdfFile), results);
+    }
+
+    @Test
+    public void findAssociatedFilesInNonExistingDirectoryFindsNothing() {
         List<String> extensions = Arrays.asList("jpg", "pdf");
         List<Path> dirs = Collections.singletonList(rootDir.resolve("asdfasdf/asdfasdf"));
         CiteKeyBasedFileFinder fileFinder = new CiteKeyBasedFileFinder(false);
-        Set<Path> results = fileFinder.findFilesByExtension(dirs, extensions);
 
-        assertEquals(Collections.emptySet(), results);
-    }
+        Collection<Path> results = fileFinder.findAssociatedFiles(entry, dirs, extensions);
 
-    @Test(expected = NullPointerException.class)
-    public void findFilesByExtensionWithNullThrowsException() {
-        CiteKeyBasedFileFinder fileFinder = new CiteKeyBasedFileFinder(false);
-        fileFinder.findFilesByExtension(Collections.emptyList(), null);
+        assertEquals(Collections.emptyList(), results);
     }
 
 }

src/test/java/org/jabref/logic/util/io/FileUtilTest.java

@@ -263,12 +263,12 @@ public void getFileExtensionWithDotAtStartReturnsEmptyExtension() {
 
     @Test
     public void getFileNameWithSimpleString() {
-        assertEquals("test", FileUtil.getFileName("test.pdf"));
+        assertEquals("test", FileUtil.getBaseName("test.pdf"));
     }
 
     @Test
     public void getFileNameWithMultipleDotsString() {
-        assertEquals("te.st", FileUtil.getFileName("te.st.PdF  "));
+        assertEquals("te.st", FileUtil.getBaseName("te.st.PdF  "));
     }
 
     @Test

src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java

@@ -2,6 +2,7 @@
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
@@ -56,7 +57,7 @@ public void testFindFiles() {
         RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[bibtexkey].*\\\\.[extension]", ',');
 
         //when
-        List<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
 
         //then
         assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/pdfInDatabase.pdf")),
@@ -72,7 +73,7 @@ public void testYearAuthFirspageFindFiles() {
         RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[year]_[auth]_[firstpage].*\\\\.[extension]", ',');
 
         //when
-        List<Path> result = fileFinder.findAssociatedFiles(entry, dirs, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(entry, dirs, extensions);
 
         //then
         assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/directory/subdirectory/2003_Hippel_209.pdf")),
@@ -94,7 +95,7 @@ public void testAuthorWithDiacritics() {
         RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[year]_[auth]_[firstpage]\\\\.[extension]", ',');
 
         //when
-        List<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
 
         //then
         assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/directory/subdirectory/2017_Gražulis_726.pdf")),
@@ -114,7 +115,7 @@ public void testFindFileInSubdirectory() {
         RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[bibtexkey].*\\\\.[extension]", ',');
 
         //when
-        List<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
 
         //then
         assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/directory/subdirectory/pdfInSubdirectory.pdf")),
@@ -134,7 +135,7 @@ public void testFindFileNonRecursive() {
         RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("*/[bibtexkey].*\\\\.[extension]", ',');
 
         //when
-        List<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
+        Collection<Path> result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions);
 
         //then
         Assert.assertTrue(result.isEmpty());