From 81feb694cf0adafb08c9fc2e224053600f5c636c Mon Sep 17 00:00:00 2001 From: u7326123 Date: Thu, 27 Oct 2022 17:07:08 +1100 Subject: [PATCH 1/4] fix the safe character replacement and auto-linking the files --- CHANGELOG.md | 1 + .../jabref/logic/util/io/CitationKeyBasedFileFinder.java | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab7b83bcc57..0d31d5d8d48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where hitting enter on the search field within the preferences dialog closed the dialog. [koppor#630](https://github.com/koppor/jabref/issues/630) - We fixed a typo within a connection error message. [koppor#625](https://github.com/koppor/jabref/issues/625) - We fixed an issue where the 'close dialog' key binding was not closing the Preferences dialog. [#8888](https://github.com/jabref/jabref/issues/8888) +- We fixed an issue when using an unsafe character in the citation key, the auto-linking feature fails to link files. [#9267](https://github.com/JabRef/jabref/issues/9267) ### Removed diff --git a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java index 27f71f6b530..7d621055322 100644 --- a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java @@ -65,8 +65,13 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li return result.stream().sorted().collect(Collectors.toList()); } + private static String getSafeCiteKey(String citeKey) { + // replace the unsafe character with the underscore. + return citeKey.replaceAll("[:/*?<>|]", "_"); + } + private boolean matches(String filename, String citeKey) { - boolean startsWithKey = filename.startsWith(citeKey); + boolean startsWithKey = filename.startsWith(getSafeCiteKey(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 From 88c46aea519b09a0dfc5b18955e4d612016845f1 Mon Sep 17 00:00:00 2001 From: u7326123 Date: Thu, 27 Oct 2022 17:46:02 +1100 Subject: [PATCH 2/4] Test the unsafe citation key attach the file. --- .../logic/util/io/CitationKeyBasedFileFinder.java | 2 +- .../util/io/CitationKeyBasedFileFinderTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java index 7d621055322..53a11deb017 100644 --- a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java @@ -65,7 +65,7 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li return result.stream().sorted().collect(Collectors.toList()); } - private static String getSafeCiteKey(String citeKey) { + public static String getSafeCiteKey(String citeKey) { // replace the unsafe character with the underscore. return citeKey.replaceAll("[:/*?<>|]", "_"); } diff --git a/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java b/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java index edb0bfaed3a..37e6a42a885 100644 --- a/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java +++ b/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java @@ -99,4 +99,17 @@ void findAssociatedFilesInNonExistingDirectoryFindsNothing() throws Exception { assertEquals(Collections.emptyList(), results); } + + @Test + void findAssociatedFilesWithUnsafeCharacters() throws Exception { + BibEntry entryWithUnsafeCitationKey = new BibEntry(StandardEntryType.Article); + entryWithUnsafeCitationKey.setCitationKey("test:test/*test?"); + + Path testFile = Files.createFile(pdfsDir.resolve("test_test__test_.pdf")); + FileFinder fileFinder = new CitationKeyBasedFileFinder(false); + + List results = fileFinder.findAssociatedFiles(entryWithUnsafeCitationKey, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); + + assertEquals(Collections.singletonList(testFile), results); + } } From 4cc19c12339d42898a6f7af0ac2c2cd97057dbb2 Mon Sep 17 00:00:00 2001 From: u7326123 Date: Thu, 27 Oct 2022 20:03:09 +1100 Subject: [PATCH 3/4] Change to citation key filter to the existing FileNameCleaner --- .../jabref/logic/util/io/CitationKeyBasedFileFinder.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java index 53a11deb017..81746101a40 100644 --- a/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java +++ b/src/main/java/org/jabref/logic/util/io/CitationKeyBasedFileFinder.java @@ -65,13 +65,8 @@ public List findAssociatedFiles(BibEntry entry, List directories, Li return result.stream().sorted().collect(Collectors.toList()); } - public static String getSafeCiteKey(String citeKey) { - // replace the unsafe character with the underscore. - return citeKey.replaceAll("[:/*?<>|]", "_"); - } - private boolean matches(String filename, String citeKey) { - boolean startsWithKey = filename.startsWith(getSafeCiteKey(citeKey)); + boolean startsWithKey = filename.startsWith(FileNameCleaner.cleanFileName(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 From 492d1508450e1317ee9539d4ae3ff0097356d24c Mon Sep 17 00:00:00 2001 From: Xianghao Wang Date: Sat, 29 Oct 2022 15:04:03 +1100 Subject: [PATCH 4/4] Add tests for "exact" and "startsWith" matches of auto-linking files --- .../io/CitationKeyBasedFileFinderTest.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java b/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java index 37e6a42a885..3f29211f4e0 100644 --- a/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java +++ b/src/test/java/org/jabref/logic/util/io/CitationKeyBasedFileFinderTest.java @@ -15,6 +15,7 @@ import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; class CitationKeyBasedFileFinderTest { @@ -101,15 +102,28 @@ void findAssociatedFilesInNonExistingDirectoryFindsNothing() throws Exception { } @Test - void findAssociatedFilesWithUnsafeCharacters() throws Exception { + void findAssociatedFilesWithUnsafeCharactersStartWithSearch() throws Exception { BibEntry entryWithUnsafeCitationKey = new BibEntry(StandardEntryType.Article); - entryWithUnsafeCitationKey.setCitationKey("test:test/*test?"); + entryWithUnsafeCitationKey.setCitationKey("?test"); - Path testFile = Files.createFile(pdfsDir.resolve("test_test__test_.pdf")); + Path testFile = Files.createFile(pdfsDir.resolve("_test_file.pdf")); FileFinder fileFinder = new CitationKeyBasedFileFinder(false); List results = fileFinder.findAssociatedFiles(entryWithUnsafeCitationKey, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); assertEquals(Collections.singletonList(testFile), results); } + + @Test + void findAssociatedFilesWithUnsafeCharactersExactSearch() throws Exception { + BibEntry entryWithUnsafeCitationKey = new BibEntry(StandardEntryType.Article); + entryWithUnsafeCitationKey.setCitationKey("test:test/*test?"); + + Path testFile = Files.createFile(pdfsDir.resolve("test_test__test_.pdf")); + FileFinder fileFinder = new CitationKeyBasedFileFinder(true); + + List results = fileFinder.findAssociatedFiles(entryWithUnsafeCitationKey, Collections.singletonList(pdfsDir), Collections.singletonList("pdf")); + + assertNotEquals(Collections.singletonList(testFile), results); + } }