Skip to content

Commit

Permalink
Fix old JabRef linked files containing the file directory as element …
Browse files Browse the repository at this point in the history
…in the filename (#9046)

* Potential fix for old File directories behavior

Fixes #8991

* TODO test

* added some tests

* Enable files/files

Co-authored-by: Christoph <siedlerkiller@gmail.com>

* changelog

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
  • Loading branch information
Siedlerchr and koppor committed Sep 3, 2022
1 parent 4685800 commit 0f4d396
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -91,6 +91,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
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
@@ -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));
}
}

0 comments on commit 0f4d396

Please sign in to comment.