Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix old JabRef linked files containing the file directory as element in the filename #9046

Merged
merged 7 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Fixed

- We fixed an issue where linked fails containing parts of the main file directory could not be opened [#8991](https://github.com/JabRef/jabref/issues/8991)
- We fixed an issue where the user could not rate an entry in the main table when an entry was not yet ranked. [#5842](https://github.com/JabRef/jabref/issues/5842)
- We fixed an issue that caused JabRef to sometimes open multiple instances when "Remote Operation" is enabled. [#8653](https://github.com/JabRef/jabref/issues/8653)
- We fixed an issue where linked files with the filetype "application/pdf" in an entry were not shown with the correct PDF-Icon in the main table [8930](https://github.com/JabRef/jabref/issues/8930)
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/org/jabref/model/util/FileHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,12 @@ public static boolean isCharLegal(char c) {
}

/**
* Converts a relative filename to an absolute one, if necessary. Returns
* an empty optional if the file does not exist.
* Converts a relative filename to an absolute one, if necessary.
*
* @param fileName the filename (e.g., a .pdf file), may contain path separators
* @param directory the directory which should be search starting point
*
* @returns an empty optional if the file does not exist, otherwise, the absolute path
*/
public static Optional<Path> find(String fileName, Path directory) {
Objects.requireNonNull(fileName);
Expand All @@ -168,6 +172,17 @@ public static Optional<Path> find(String fileName, Path directory) {
}

Path resolvedFile = directory.resolve(fileName);
if (Files.exists(resolvedFile)) {
return Optional.of(resolvedFile);
}

// get the furthest path element from root and check if our filename starts with the same name
// workaround for old JabRef behavior
String furthestDirFromRoot = directory.getFileName().toString();
if (fileName.startsWith(furthestDirFromRoot)) {
resolvedFile = directory.resolveSibling(fileName);
}

if (Files.exists(resolvedFile)) {
return Optional.of(resolvedFile);
} else {
Expand Down
37 changes: 34 additions & 3 deletions src/test/java/org/jabref/model/util/FileHelperTest.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
package org.jabref.model.util;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Optional;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.junit.jupiter.api.Assertions.assertEquals;

class FileHelperTest {

@Test
public void extractFileExtension() {
final String filePath = FileHelperTest.class.getResource("pdffile.pdf").getPath();
Expand All @@ -24,14 +27,42 @@ public void fileExtensionFromUrl() {

@Test
public void testFileNameEmpty() {
Path path = Path.of("/");
assertEquals(Optional.of(path), FileHelper.find("", path));
Path path = Path.of("/");
assertEquals(Optional.of(path), FileHelper.find("", path));
}

@ParameterizedTest
@ValueSource(strings = { "*", "?", ">", "\"" })
@ValueSource(strings = {"*", "?", ">", "\""})
public void testFileNameIllegal(String fileName) {
Path path = Path.of("/");
assertEquals(Optional.empty(), FileHelper.find(fileName, path));
}

@Test
public void testFindsFileInDirectory(@TempDir Path temp) throws Exception {
Path firstFilePath = temp.resolve("files");
Files.createDirectories(firstFilePath);
Path firstFile = Files.createFile(firstFilePath.resolve("test.pdf"));

assertEquals(Optional.of(firstFile), FileHelper.find("test.pdf", temp.resolve("files")));
}

@Test
public void testFindsFileStartingWithTheSameDirectory(@TempDir Path temp) throws Exception {
Path firstFilePath = temp.resolve("files");
Files.createDirectories(firstFilePath);
Path firstFile = Files.createFile(firstFilePath.resolve("test.pdf"));

assertEquals(Optional.of(firstFile), FileHelper.find("files/test.pdf", temp.resolve("files")));
}

@Test
public void testDoesNotFindsFileStartingWithTheSameDirectoryHasASubdirectory(@TempDir Path temp) throws Exception {
Path firstFilesPath = temp.resolve("files");
Path secondFilesPath = firstFilesPath.resolve("files");
Files.createDirectories(secondFilesPath);
Path testFile = secondFilesPath.resolve("test.pdf");
Files.createFile(testFile);
assertEquals(Optional.of(testFile.toAbsolutePath()), FileHelper.find("files/test.pdf", firstFilesPath));
}
}