From 26c4462df1b3ce645b0fd13c57db9dfcf38c3522 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 10 Dec 2017 14:42:48 +0100 Subject: [PATCH 1/8] Refactor auto link classes --- .../externalfiles/AutoSetFileLinksUtil.java | 32 ++++++++++++------- .../gui/externalfiles/AutoSetLinks.java | 4 +-- .../LinkedFilesEditorViewModel.java | 5 ++- .../logic/util/io/CiteKeyBasedFileFinder.java | 14 +++----- .../org/jabref/logic/util/io/FileFinder.java | 11 ++++--- .../logic/util/io/RegExpBasedFileFinder.java | 10 +++--- .../AutoSetFileLinksUtilTest.java | 7 ++-- 7 files changed, 43 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java index 0fa1fce83f3..176eab0351d 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java @@ -26,22 +26,33 @@ public class AutoSetFileLinksUtil { private static final Log LOGGER = LogFactory.getLog(AutoSetLinks.class); + private List directories; + private AutoLinkPreferences autoLinkPreferences; + private ExternalFileTypes externalFileTypes; - public List findassociatedNotLinkedFiles(BibEntry entry, BibDatabaseContext databaseContext, FileDirectoryPreferences fileDirPrefs, AutoLinkPreferences autoLinkPrefs, ExternalFileTypes externalFileTypes) { + public AutoSetFileLinksUtil(BibDatabaseContext databaseContext, FileDirectoryPreferences fileDirectoryPreferences, AutoLinkPreferences autoLinkPreferences, ExternalFileTypes externalFileTypes) { + this(databaseContext.getFileDirectoriesAsPaths(fileDirectoryPreferences), autoLinkPreferences, externalFileTypes); + } + + public AutoSetFileLinksUtil(List directories, AutoLinkPreferences autoLinkPreferences, ExternalFileTypes externalFileTypes) { + this.directories = directories; + this.autoLinkPreferences = autoLinkPreferences; + this.externalFileTypes = externalFileTypes; + } + + public List findAssociatedNotLinkedFiles(BibEntry entry) { List linkedFiles = new ArrayList<>(); - List dirs = databaseContext.getFileDirectoriesAsPaths(fileDirPrefs); List extensions = externalFileTypes.getExternalFileTypeSelection().stream().map(ExternalFileType::getExtension).collect(Collectors.toList()); - // Run the search operation: - FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPrefs); - List result = fileFinder.findAssociatedFiles(entry, dirs, extensions); - - // Iterate over the entries: + // Run the search operation + FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPreferences); + List result = fileFinder.findAssociatedFiles(entry, directories, extensions); + // Collect the found files that are not yet linked for (Path foundFile : result) { - boolean existingSameFile = entry.getFiles().stream() - .map(file -> file.findIn(dirs)) + boolean fileAlreadyLinked = entry.getFiles().stream() + .map(file -> file.findIn(directories)) .anyMatch(file -> { try { return file.isPresent() && Files.isSameFile(file.get(), foundFile); @@ -50,8 +61,7 @@ public List findassociatedNotLinkedFiles(BibEntry entry, BibDatabase } return false; }); - if (!existingSameFile) { - + if (!fileAlreadyLinked) { Optional type = FileHelper.getFileExtension(foundFile) .map(externalFileTypes::getExternalFileTypeByExt) .orElse(Optional.of(new UnknownExternalFileType(""))); diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java index b8f15487205..c26042cc3f3 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java @@ -82,11 +82,11 @@ public static Runnable autoSetLinks(final List entries, final NamedCom Runnable r = () -> { boolean foundAny = false; - AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); for (BibEntry entry : entries) { - List linkedFiles = util.findassociatedNotLinkedFiles(entry, databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); + List linkedFiles = util.findAssociatedNotLinkedFiles(entry); if (ce != null) { for (LinkedFile linkedFile : linkedFiles) { diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index 5860b731501..5a273f4419f 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -158,9 +158,8 @@ public void bindToEntry(BibEntry entry) { private List findAssociatedNotLinkedFiles(BibEntry entry) { List result = new ArrayList<>(); - AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); - List linkedFiles = util.findassociatedNotLinkedFiles(entry, databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); - + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); + List linkedFiles = util.findAssociatedNotLinkedFiles(entry); for (LinkedFile linkedFile : linkedFiles) { LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext); newLinkedFile.markAsAutomaticallyFound(); diff --git a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java index 462250d88f1..d3e03580133 100644 --- a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java @@ -4,11 +4,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; -import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -19,6 +16,8 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.util.FileHelper; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -32,20 +31,15 @@ class CiteKeyBasedFileFinder implements FileFinder { } @Override - public Map> findAssociatedFiles(List entries, List directories, List extensions) { + public Multimap findAssociatedFiles(List entries, List directories, List extensions) { Objects.requireNonNull(directories); Objects.requireNonNull(entries); - Map> result = new HashMap<>(); + Multimap result = ArrayListMultimap.create(); // First scan directories Set filesWithExtension = findFilesByExtension(directories, extensions); - // Initialize Result-Set - for (BibEntry entry : entries) { - result.put(entry, new ArrayList<>()); - } - // Now look for keys nextFile: for (Path file : filesWithExtension) { diff --git a/src/main/java/org/jabref/logic/util/io/FileFinder.java b/src/main/java/org/jabref/logic/util/io/FileFinder.java index 96abd65c6a2..ff623aed520 100644 --- a/src/main/java/org/jabref/logic/util/io/FileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/FileFinder.java @@ -1,12 +1,14 @@ package org.jabref.logic.util.io; import java.nio.file.Path; +import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; import org.jabref.model.entry.BibEntry; +import com.google.common.collect.Multimap; + public interface FileFinder { /** @@ -17,10 +19,9 @@ public interface FileFinder { * @param directories The root directories to search. * @param extensions The extensions that are acceptable. */ - Map> findAssociatedFiles(List entries, List directories, List extensions); + Multimap findAssociatedFiles(List entries, List directories, List extensions); - default List findAssociatedFiles(BibEntry entry, List directories, List extensions) { - Map> associatedFiles = findAssociatedFiles(Collections.singletonList(entry), directories, extensions); - return associatedFiles.getOrDefault(entry, Collections.emptyList()); + default Collection findAssociatedFiles(BibEntry entry, List directories, List extensions) { + return findAssociatedFiles(Collections.singletonList(entry), directories, extensions).get(entry); } } diff --git a/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java index ec60583e3b2..2ea0203a2d2 100644 --- a/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java @@ -6,9 +6,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -18,6 +16,8 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.strings.StringUtil; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -65,10 +65,10 @@ public static String expandBrackets(String bracketString, BibEntry entry, BibDat } @Override - public Map> findAssociatedFiles(List entries, List directories, List extensions) { - Map> res = new HashMap<>(); + public Multimap findAssociatedFiles(List entries, List directories, List extensions) { + Multimap res = ArrayListMultimap.create(); for (BibEntry entry : entries) { - res.put(entry, findFiles(entry, extensions, directories)); + res.putAll(entry, findFiles(entry, extensions, directories)); } return res; } diff --git a/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java b/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java index 8bf55431ced..34bbfbe1821 100644 --- a/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java +++ b/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java @@ -38,8 +38,7 @@ public void setUp() throws Exception { entry.setCiteKey("CiteKey"); file = folder.newFile("CiteKey.pdf").toPath(); when(databaseContext.getFileDirectoriesAsPaths(any())).thenReturn(Collections.singletonList(folder.getRoot().toPath())); - when(externalFileTypes.getExternalFileTypeSelection()).thenReturn(new TreeSet<>(externalFileTypes.getDefaultExternalFileTypes())); - + when(externalFileTypes.getExternalFileTypeSelection()).thenReturn(new TreeSet<>(ExternalFileTypes.getDefaultExternalFileTypes())); } @Test @@ -47,8 +46,8 @@ public void test() { //Due to mocking the externalFileType class, the file extension will not be found List expected = Collections.singletonList(new LinkedFile("", file.toString(), "")); - AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); - List actual = util.findassociatedNotLinkedFiles(entry, databaseContext, fileDirPrefs, autoLinkPrefs, externalFileTypes); + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, fileDirPrefs, autoLinkPrefs, externalFileTypes); + List actual = util.findAssociatedNotLinkedFiles(entry); assertEquals(expected, actual); } From 456241c1624f549dc0b1453114b2fe0143ef2212 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 10 Dec 2017 15:39:53 +0100 Subject: [PATCH 2/8] Fixes #3472: Files starting with BibTeX key of a similar entry are no longer found by mistake --- CHANGELOG.md | 1 + src/main/java/org/jabref/gui/BasePanel.java | 4 +- .../externalfiles/AutoSetFileLinksUtil.java | 3 +- .../BibtexKeyPatternUtil.java | 2 +- .../org/jabref/logic/net/URLDownload.java | 2 +- .../logic/util/io/CiteKeyBasedFileFinder.java | 28 +++++-- .../org/jabref/logic/util/io/FileUtil.java | 12 +-- .../org/jabref/model/strings/StringUtil.java | 4 + src/test/java/org/jabref/BibtexTestData.java | 2 +- .../util/io/CiteKeyBasedFileFinderTest.java | 74 ++++++++++--------- .../jabref/logic/util/io/FileUtilTest.java | 4 +- .../util/io/RegExpBasedFileFinderTests.java | 11 +-- 12 files changed, 84 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a45707b6851..c5bc0890522 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index 7020faa005c..6efaa4abe24 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/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 files = fileFinder.findAssociatedFiles(entry, dirs, extensions); + Collection files = fileFinder.findAssociatedFiles(entry, dirs, extensions); if (!files.isEmpty()) { - Path file = files.get(0); + Path file = files.iterator().next(); Optional type = ExternalFileTypes.getInstance().getExternalFileTypeByFile(file); if (type.isPresent()) { try { diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java index 176eab0351d..ced68897fec 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java +++ b/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 findAssociatedNotLinkedFiles(BibEntry entry) { // Run the search operation FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPreferences); - List result = fileFinder.findAssociatedFiles(entry, directories, extensions); + Collection result = fileFinder.findAssociatedFiles(entry, directories, extensions); // Collect the found files that are not yet linked for (Path foundFile : result) { diff --git a/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java b/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java index 72e404352fd..0f8723c35f0 100644 --- a/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java +++ b/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() { } diff --git a/src/main/java/org/jabref/logic/net/URLDownload.java b/src/main/java/org/jabref/logic/net/URLDownload.java index cec9c307c8c..b79d878f2df 100644 --- a/src/main/java/org/jabref/logic/net/URLDownload.java +++ b/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 diff --git a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java index d3e03580133..cce28977b01 100644 --- a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java +++ b/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 findAssociatedFiles(List 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 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 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 findAssociatedFiles(List 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 findFilesByExtension(List directories, List extensions) { + private Set findFilesByExtension(List directories, List extensions) { Objects.requireNonNull(extensions, "Extensions must not be null!"); BiPredicate isFileWithCorrectExtension = (path, attributes) -> diff --git a/src/main/java/org/jabref/logic/util/io/FileUtil.java b/src/main/java/org/jabref/logic/util/io/FileUtil.java index 5a734fd47ff..d5dcfa139f7 100644 --- a/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/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 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 extension = getFileExtension(fileName); diff --git a/src/main/java/org/jabref/model/strings/StringUtil.java b/src/main/java/org/jabref/model/strings/StringUtil.java index d9cf81c2172..cb2c647355c 100644 --- a/src/main/java/org/jabref/model/strings/StringUtil.java +++ b/src/main/java/org/jabref/model/strings/StringUtil.java @@ -719,4 +719,8 @@ public static List 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; + } } diff --git a/src/test/java/org/jabref/BibtexTestData.java b/src/test/java/org/jabref/BibtexTestData.java index acd12667ecb..0ca8e4cd27f 100644 --- a/src/test/java/org/jabref/BibtexTestData.java +++ b/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" diff --git a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java b/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java index fde2862109d..9bb860e24aa 100644 --- a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java +++ b/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 extensions = Arrays.asList("jpg", "pdf"); - List dirs = Arrays.asList(rootDir.resolve("graphicsDir"), rootDir.resolve("pdfs")); + List dirs = Arrays.asList(graphicsDir, pdfsDir); FileFinder fileFinder = new CiteKeyBasedFileFinder(false); - List 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 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 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 results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); + + assertEquals(Arrays.asList(pdfFile, secondPdfFile), results); + } + + @Test + public void findAssociatedFilesInNonExistingDirectoryFindsNothing() { List extensions = Arrays.asList("jpg", "pdf"); List dirs = Collections.singletonList(rootDir.resolve("asdfasdf/asdfasdf")); CiteKeyBasedFileFinder fileFinder = new CiteKeyBasedFileFinder(false); - Set results = fileFinder.findFilesByExtension(dirs, extensions); - assertEquals(Collections.emptySet(), results); - } + Collection 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); } } diff --git a/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 3a9a424a3d3..78aa63ac099 100644 --- a/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/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 diff --git a/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java b/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java index 61d1d088b3e..bd811d31618 100644 --- a/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java +++ b/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 result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + Collection 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 result = fileFinder.findAssociatedFiles(entry, dirs, extensions); + Collection 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 result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + Collection 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 result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + Collection 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 result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + Collection result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); //then Assert.assertTrue(result.isEmpty()); From 3784d907751b6204dabcf95ce519e289c97b39fb Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 10 Dec 2017 19:39:25 +0100 Subject: [PATCH 3/8] Simplify code --- src/main/java/org/jabref/gui/BasePanel.java | 4 +-- .../externalfiles/AutoSetFileLinksUtil.java | 3 +- .../logic/util/io/CiteKeyBasedFileFinder.java | 34 +++++++++---------- .../org/jabref/logic/util/io/FileFinder.java | 12 ++----- .../logic/util/io/RegExpBasedFileFinder.java | 14 ++------ .../util/io/CiteKeyBasedFileFinderTest.java | 12 +++---- 6 files changed, 28 insertions(+), 51 deletions(-) diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index 6efaa4abe24..7020faa005c 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/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()); - Collection files = fileFinder.findAssociatedFiles(entry, dirs, extensions); + List files = fileFinder.findAssociatedFiles(entry, dirs, extensions); if (!files.isEmpty()) { - Path file = files.iterator().next(); + Path file = files.get(0); Optional type = ExternalFileTypes.getInstance().getExternalFileTypeByFile(file); if (type.isPresent()) { try { diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java index ced68897fec..176eab0351d 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java @@ -4,7 +4,6 @@ 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; @@ -48,7 +47,7 @@ public List findAssociatedNotLinkedFiles(BibEntry entry) { // Run the search operation FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPreferences); - Collection result = fileFinder.findAssociatedFiles(entry, directories, extensions); + List result = fileFinder.findAssociatedFiles(entry, directories, extensions); // Collect the found files that are not yet linked for (Path foundFile : result) { diff --git a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java index cce28977b01..11782cb2960 100644 --- a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java @@ -4,6 +4,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -18,8 +20,6 @@ import org.jabref.model.strings.StringUtil; import org.jabref.model.util.FileHelper; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Multimap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -33,37 +33,35 @@ class CiteKeyBasedFileFinder implements FileFinder { } @Override - public Multimap findAssociatedFiles(List entries, List directories, List extensions) { + public List findAssociatedFiles(BibEntry entry, List directories, List extensions) { Objects.requireNonNull(directories); - Objects.requireNonNull(entries); + Objects.requireNonNull(entry); - Multimap result = ArrayListMultimap.create(); + Optional citeKeyOptional = entry.getCiteKeyOptional(); + if (StringUtil.isBlank(citeKeyOptional)) { + return Collections.emptyList(); + } + String citeKey = citeKeyOptional.get(); + + List result = new ArrayList<>(); // First scan directories Set filesWithExtension = findFilesByExtension(directories, extensions); // Now look for keys - nextFile: for (Path file : filesWithExtension) { String name = file.getFileName().toString(); String nameWithoutExtension = FileUtil.getBaseName(name); // First, look for exact matches - for (BibEntry entry : entries) { - Optional citeKey = entry.getCiteKeyOptional(); - if (StringUtil.isNotBlank(citeKey) && nameWithoutExtension.equals(citeKey.get())) { - result.get(entry).add(file); - continue nextFile; - } + if (nameWithoutExtension.equals(citeKey)) { + 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) { - for (BibEntry entry : entries) { - Optional citeKey = entry.getCiteKeyOptional(); - if (StringUtil.isNotBlank(citeKey) && matches(name, citeKey.get())) { - result.get(entry).add(file); - continue nextFile; - } + if (matches(name, citeKey)) { + result.add(file); } } } diff --git a/src/main/java/org/jabref/logic/util/io/FileFinder.java b/src/main/java/org/jabref/logic/util/io/FileFinder.java index ff623aed520..fd6e6bf9c69 100644 --- a/src/main/java/org/jabref/logic/util/io/FileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/FileFinder.java @@ -1,27 +1,19 @@ package org.jabref.logic.util.io; import java.nio.file.Path; -import java.util.Collection; -import java.util.Collections; import java.util.List; import org.jabref.model.entry.BibEntry; -import com.google.common.collect.Multimap; - public interface FileFinder { /** * Finds all files in the given directories that are probably associated with the given entries and have one of the * passed extensions. * - * @param entries The entries to search for. + * @param entry The entry to search files for. * @param directories The root directories to search. * @param extensions The extensions that are acceptable. */ - Multimap findAssociatedFiles(List entries, List directories, List extensions); - - default Collection findAssociatedFiles(BibEntry entry, List directories, List extensions) { - return findAssociatedFiles(Collections.singletonList(entry), directories, extensions).get(entry); - } + List findAssociatedFiles(BibEntry entry, List directories, List extensions); } diff --git a/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java index 2ea0203a2d2..1d8396e02e5 100644 --- a/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java @@ -16,8 +16,6 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.strings.StringUtil; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Multimap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -64,15 +62,6 @@ public static String expandBrackets(String bracketString, BibEntry entry, BibDat return expandedStringBuffer.toString(); } - @Override - public Multimap findAssociatedFiles(List entries, List directories, List extensions) { - Multimap res = ArrayListMultimap.create(); - for (BibEntry entry : entries) { - res.putAll(entry, findFiles(entry, extensions, directories)); - } - return res; - } - /** * Method for searching for files using regexp. A list of extensions and directories can be * given. @@ -81,7 +70,8 @@ public Multimap findAssociatedFiles(List entries, List * @param directories The root directories to search. * @return A list of files paths matching the given criteria. */ - private List findFiles(BibEntry entry, List extensions, List directories) { + @Override + public List findAssociatedFiles(BibEntry entry, List directories, List extensions) { String extensionRegExp = '(' + String.join("|", extensions) + ')'; return findFile(entry, directories, extensionRegExp); } diff --git a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java b/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java index 9bb860e24aa..d407937f7b5 100644 --- a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java +++ b/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java @@ -4,10 +4,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.BibtexEntryTypes; @@ -72,9 +70,9 @@ public void findAssociatedFilesInSubDirectories() { List dirs = Arrays.asList(graphicsDir, pdfsDir); FileFinder fileFinder = new CiteKeyBasedFileFinder(false); - Collection results = fileFinder.findAssociatedFiles(entry, dirs, extensions); + List results = fileFinder.findAssociatedFiles(entry, dirs, extensions); - assertEquals(Arrays.asList(jpgFile, pdfFile), results.stream().sorted().collect(Collectors.toList())); + assertEquals(Arrays.asList(jpgFile, pdfFile), results); } @Test @@ -82,7 +80,7 @@ public void findAssociatedFilesIgnoresFilesStartingWithKeyButContinueWithText() Files.createFile(pdfsDir.resolve("HipKro03a - Hello second paper.pdf")); FileFinder fileFinder = new CiteKeyBasedFileFinder(false); - Collection results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); + List results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); assertEquals(Collections.singletonList(pdfFile), results); } @@ -92,7 +90,7 @@ public void findAssociatedFilesFindsFilesStartingWithKey() throws Exception { Path secondPdfFile = Files.createFile(pdfsDir.resolve("HipKro03_Hello second paper.pdf")); FileFinder fileFinder = new CiteKeyBasedFileFinder(false); - Collection results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); + List results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); assertEquals(Arrays.asList(pdfFile, secondPdfFile), results); } @@ -103,7 +101,7 @@ public void findAssociatedFilesInNonExistingDirectoryFindsNothing() { List dirs = Collections.singletonList(rootDir.resolve("asdfasdf/asdfasdf")); CiteKeyBasedFileFinder fileFinder = new CiteKeyBasedFileFinder(false); - Collection results = fileFinder.findAssociatedFiles(entry, dirs, extensions); + List results = fileFinder.findAssociatedFiles(entry, dirs, extensions); assertEquals(Collections.emptyList(), results); } From 18a6688bafa47088033d47d7f71495185f0dbbcd Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 10 Dec 2017 23:56:44 +0100 Subject: [PATCH 4/8] Implement feedback, fix tests and checkstyle --- .../logic/bibtexkeypattern/BibtexKeyPatternUtil.java | 3 +-- .../jabref/logic/util/io/CiteKeyBasedFileFinder.java | 2 +- src/test/java/org/jabref/CodeStyleTests.java | 2 +- .../logic/util/io/CiteKeyBasedFileFinderTest.java | 2 +- .../logic/util/io/RegExpBasedFileFinderTests.java | 11 +++++------ 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java b/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java index 0f8723c35f0..97bde0d9fd1 100644 --- a/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java +++ b/src/main/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtil.java @@ -19,10 +19,9 @@ * This is the utility class of the LabelPattern package. */ 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: public static final String CHARS = "abcdefghijklmnopqrstuvwxyz"; + private static final Log LOGGER = LogFactory.getLog(BibtexKeyPatternUtil.class); private BibtexKeyPatternUtil() { } diff --git a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java index 11782cb2960..cb734a1b5cd 100644 --- a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java @@ -66,7 +66,7 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li } } - return result; + return result.stream().sorted().collect(Collectors.toList()); } private boolean matches(String filename, String citeKey) { diff --git a/src/test/java/org/jabref/CodeStyleTests.java b/src/test/java/org/jabref/CodeStyleTests.java index ec6a5641437..7d778afc855 100644 --- a/src/test/java/org/jabref/CodeStyleTests.java +++ b/src/test/java/org/jabref/CodeStyleTests.java @@ -19,6 +19,6 @@ public void StringUtilClassIsSmall() throws Exception { Assert.assertTrue("StringUtil increased in size. " + "We try to keep this class as small as possible. " - + "Thus think twice if you add something to StringUtil.", lineCount <= 722); + + "Thus think twice if you add something to StringUtil.", lineCount <= 726); } } diff --git a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java b/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java index d407937f7b5..7428070109c 100644 --- a/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java +++ b/src/test/java/org/jabref/logic/util/io/CiteKeyBasedFileFinderTest.java @@ -92,7 +92,7 @@ public void findAssociatedFilesFindsFilesStartingWithKey() throws Exception { List results = fileFinder.findAssociatedFiles(entry, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); - assertEquals(Arrays.asList(pdfFile, secondPdfFile), results); + assertEquals(Arrays.asList(secondPdfFile, pdfFile), results); } @Test diff --git a/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java b/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java index bd811d31618..61d1d088b3e 100644 --- a/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java +++ b/src/test/java/org/jabref/logic/util/io/RegExpBasedFileFinderTests.java @@ -2,7 +2,6 @@ import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Collection; import java.util.Collections; import java.util.List; @@ -57,7 +56,7 @@ public void testFindFiles() { RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[bibtexkey].*\\\\.[extension]", ','); //when - Collection result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + List result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); //then assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/pdfInDatabase.pdf")), @@ -73,7 +72,7 @@ public void testYearAuthFirspageFindFiles() { RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[year]_[auth]_[firstpage].*\\\\.[extension]", ','); //when - Collection result = fileFinder.findAssociatedFiles(entry, dirs, extensions); + List 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")), @@ -95,7 +94,7 @@ public void testAuthorWithDiacritics() { RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[year]_[auth]_[firstpage]\\\\.[extension]", ','); //when - Collection result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + List 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")), @@ -115,7 +114,7 @@ public void testFindFileInSubdirectory() { RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("**/[bibtexkey].*\\\\.[extension]", ','); //when - Collection result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + List result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); //then assertEquals(Collections.singletonList(Paths.get("src/test/resources/org/jabref/logic/importer/unlinkedFilesTestFolder/directory/subdirectory/pdfInSubdirectory.pdf")), @@ -135,7 +134,7 @@ public void testFindFileNonRecursive() { RegExpBasedFileFinder fileFinder = new RegExpBasedFileFinder("*/[bibtexkey].*\\\\.[extension]", ','); //when - Collection result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); + List result = fileFinder.findAssociatedFiles(localEntry, dirs, extensions); //then Assert.assertTrue(result.isEmpty()); From fb207b23881afaa729802b48fdaded7d8d337ef7 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 23 Dec 2017 13:56:58 +0100 Subject: [PATCH 5/8] Minor changes --- .../java/org/jabref/gui/entryeditor/EntryEditorTab.java | 3 ++- .../org/jabref/logic/util/io/CiteKeyBasedFileFinder.java | 6 ++---- src/main/java/org/jabref/model/strings/StringUtil.java | 4 ---- src/test/java/org/jabref/CodeStyleTests.java | 2 +- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditorTab.java b/src/main/java/org/jabref/gui/entryeditor/EntryEditorTab.java index 652cfcf2e3c..ebc4fef4629 100644 --- a/src/main/java/org/jabref/gui/entryeditor/EntryEditorTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/EntryEditorTab.java @@ -2,6 +2,7 @@ import javafx.scene.control.Tab; +import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.model.entry.BibEntry; public abstract class EntryEditorTab extends Tab { @@ -32,7 +33,7 @@ protected void handleFocus() { public void notifyAboutFocus(BibEntry entry) { if (!entry.equals(currentEntry)) { currentEntry = entry; - bindToEntry(entry); + DefaultTaskExecutor.runInJavaFXThread(() -> bindToEntry(entry)); } handleFocus(); } diff --git a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java index cb734a1b5cd..c00caa994d5 100644 --- a/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CiteKeyBasedFileFinder.java @@ -59,10 +59,8 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li continue; } // If we get here, we did not find any exact matches. If non-exact matches are allowed, try to find one - if (!exactKeyOnly) { - if (matches(name, citeKey)) { + if (!exactKeyOnly && matches(name, citeKey)) { result.add(file); - } } } @@ -75,7 +73,7 @@ private boolean matches(String filename, String citeKey) { // 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 !BibtexKeyPatternUtil.CHARS.contains(Character.toString(charAfterKey)); } return false; } diff --git a/src/main/java/org/jabref/model/strings/StringUtil.java b/src/main/java/org/jabref/model/strings/StringUtil.java index cb2c647355c..d9cf81c2172 100644 --- a/src/main/java/org/jabref/model/strings/StringUtil.java +++ b/src/main/java/org/jabref/model/strings/StringUtil.java @@ -719,8 +719,4 @@ public static List 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; - } } diff --git a/src/test/java/org/jabref/CodeStyleTests.java b/src/test/java/org/jabref/CodeStyleTests.java index 7d778afc855..ec6a5641437 100644 --- a/src/test/java/org/jabref/CodeStyleTests.java +++ b/src/test/java/org/jabref/CodeStyleTests.java @@ -19,6 +19,6 @@ public void StringUtilClassIsSmall() throws Exception { Assert.assertTrue("StringUtil increased in size. " + "We try to keep this class as small as possible. " - + "Thus think twice if you add something to StringUtil.", lineCount <= 726); + + "Thus think twice if you add something to StringUtil.", lineCount <= 722); } } From c45de80335c51022bd887d443a9fa8ed503b8f74 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 23 Dec 2017 14:03:26 +0100 Subject: [PATCH 6/8] Fix changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60e6238698f..6669379b10e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - 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 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) + - We fixed an issue where trying to fetch data from Medline/PubMed resulted in an error. [#3463](https://github.com/JabRef/jabref/issues/3463) + - We fixed an issue where double clicking on an entry in the integrity check dialog resulted in an exception. [#3485](https://github.com/JabRef/jabref/issues/3485) - We fixed an issue where the entry type could sometimes not be changed when the entry editor was open [#3435](https://github.com/JabRef/jabref/issues/3435) - We fixed an issue where dropping a pdf on the entry table and renaming it triggered an exception. [#3490](https://github.com/JabRef/jabref/issues/3490) - We fixed an issue where no longer existing files could not be removed from the entry by pressing the del key. [#3493](https://github.com/JabRef/jabref/issues/3493) From e661b76e79886fae6b6e1e2ec06a38839c367e42 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 23 Dec 2017 14:04:20 +0100 Subject: [PATCH 7/8] Really fix changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6669379b10e..9877e642c88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - 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) - We fixed an issue where trying to fetch data from Medline/PubMed resulted in an error. [#3463](https://github.com/JabRef/jabref/issues/3463) From 9d039226ffa6997532bf4254d89eec90c4d59227 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sat, 23 Dec 2017 14:42:48 +0100 Subject: [PATCH 8/8] Fix build --- .../org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java index 0655f40a6fe..95982cf8074 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java @@ -68,8 +68,7 @@ public List findAssociatedNotLinkedFiles(BibEntry entry) { .orElse(Optional.of(new UnknownExternalFileType(""))); String strType = type.isPresent() ? type.get().getName() : ""; - String relativeFilePath = FileUtil.shortenFileName(foundFile, dirs) - .toString(); + String relativeFilePath = FileUtil.shortenFileName(foundFile, directories).toString(); LinkedFile linkedFile = new LinkedFile("", relativeFilePath, strType); linkedFiles.add(linkedFile); }