Skip to content

Commit

Permalink
move check for invalid characters to linked files editor (#12219)
Browse files Browse the repository at this point in the history
* move check for invalid characters to linked files editor

Fixes #12212

some cleanup

* fix checkstyle

* restore switch

* set working dir path

* checkstyle

* add test

* fix import
  • Loading branch information
Siedlerchr authored Nov 23, 2024
1 parent 38b70e2 commit 4f8ece3
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ public Parent getNode() {

@FXML
private void addNewFile() {
dialogService.showCustomDialogAndWait(new LinkedFileEditDialog()).ifPresent(newLinkedFile -> {
dialogService.showCustomDialogAndWait(new LinkedFileEditDialog()).filter(file -> !file.isEmpty()).ifPresent(newLinkedFile -> {
viewModel.addNewLinkedFile(newLinkedFile);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -32,15 +31,13 @@
import org.jabref.gui.linkedfile.AttachFileFromURLAction;
import org.jabref.gui.preferences.GuiPreferences;
import org.jabref.gui.util.BindingsHelper;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.bibtex.FileFieldWriter;
import org.jabref.logic.importer.FulltextFetchers;
import org.jabref.logic.importer.util.FileFieldParser;
import org.jabref.logic.integrity.FieldCheckers;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.BackgroundTask;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.logic.util.io.FileNameCleaner;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -107,19 +104,6 @@ public static LinkedFile fromFile(Path file, List<Path> fileDirectories, Externa
return new LinkedFile("", relativePath, suggestedFileType.getName());
}

public LinkedFileViewModel fromFile(Path file, ExternalApplicationsPreferences externalApplicationsPreferences) {
List<Path> fileDirectories = databaseContext.getFileDirectories(preferences.getFilePreferences());

LinkedFile linkedFile = fromFile(file, fileDirectories, externalApplicationsPreferences);
return new LinkedFileViewModel(
linkedFile,
entry,
databaseContext,
taskExecutor,
dialogService,
preferences);
}

private List<LinkedFileViewModel> parseToFileViewModel(String stringValue) {
return FileFieldParser.parse(stringValue).stream()
.map(linkedFile -> new LinkedFileViewModel(
Expand All @@ -140,41 +124,6 @@ public ListProperty<LinkedFileViewModel> filesProperty() {
return files;
}

public void addNewFile() {
Path workingDirectory = databaseContext.getFirstExistingFileDir(preferences.getFilePreferences())
.orElse(preferences.getFilePreferences().getWorkingDirectory());

FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.withInitialDirectory(workingDirectory)
.build();

List<Path> fileDirectories = databaseContext.getFileDirectories(preferences.getFilePreferences());
List<Path> selectedFiles = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration);

for (Path fileToAdd : selectedFiles) {
if (FileUtil.detectBadFileName(fileToAdd.toString())) {
String newFilename = FileNameCleaner.cleanFileName(fileToAdd.getFileName().toString());

boolean correctButtonPressed = dialogService.showConfirmationDialogAndWait(Localization.lang("File \"%0\" cannot be added!", fileToAdd.getFileName()),
Localization.lang("Illegal characters in the file name detected.\nFile will be renamed to \"%0\" and added.", newFilename),
Localization.lang("Rename and add"));

if (correctButtonPressed) {
Path correctPath = fileToAdd.resolveSibling(newFilename);
try {
Files.move(fileToAdd, correctPath);
addNewLinkedFile(correctPath, fileDirectories);
} catch (IOException ex) {
LOGGER.error("Error moving file", ex);
dialogService.showErrorDialogAndWait(ex);
}
}
} else {
addNewLinkedFile(fileToAdd, fileDirectories);
}
}
}

public void addNewLinkedFile(LinkedFile linkedFile) {
files.add(new LinkedFileViewModel(
linkedFile,
Expand All @@ -185,22 +134,17 @@ public void addNewLinkedFile(LinkedFile linkedFile) {
preferences));
}

private void addNewLinkedFile(Path correctPath, List<Path> fileDirectories) {
LinkedFile newLinkedFile = fromFile(correctPath, fileDirectories, preferences.getExternalApplicationsPreferences());
addNewLinkedFile(newLinkedFile);
}

@Override
public void bindToEntry(BibEntry entry) {
super.bindToEntry(entry);

if ((entry != null) && preferences.getEntryEditorPreferences().autoLinkFilesEnabled()) {
if (preferences.getEntryEditorPreferences().autoLinkFilesEnabled()) {
LOGGER.debug("Auto-linking files for entry {}", entry);
BackgroundTask<List<LinkedFileViewModel>> findAssociatedNotLinkedFiles = BackgroundTask
.wrap(() -> findAssociatedNotLinkedFiles(entry))
.onSuccess(list -> {
if (!list.isEmpty()) {
LOGGER.debug("Found non-associated files:", list);
LOGGER.debug("Found non-associated files: {}", list);
files.addAll(list);
}
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.jabref.gui.linkedfile;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Optional;
import java.util.regex.Pattern;
Expand All @@ -22,15 +24,22 @@
import org.jabref.gui.frame.ExternalApplicationsPreferences;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.FilePreferences;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.io.FileNameCleaner;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.LinkedFile;

import com.google.common.annotations.VisibleForTesting;
import com.tobiasdiez.easybind.EasyBind;
import com.tobiasdiez.easybind.optional.ObservableOptionalValue;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class LinkedFileEditDialogViewModel extends AbstractViewModel {

private static final Logger LOGGER = LoggerFactory.getLogger(LinkedFileEditDialogViewModel.class);

private static final Pattern REMOTE_LINK_PATTERN = Pattern.compile("[a-z]+://.*");
private final StringProperty link = new SimpleStringProperty("");
private final StringProperty description = new SimpleStringProperty("");
Expand Down Expand Up @@ -86,13 +95,31 @@ public void openBrowseDialog() {
.withInitialFileName(fileName)
.build();

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(path -> {
// Store the directory for next time:
filePreferences.setWorkingDirectory(path);
link.set(relativize(path));
dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(this::checkForBadFileNameAndAdd);
}

setExternalFileTypeByExtension(link.getValueSafe());
});
@VisibleForTesting
void checkForBadFileNameAndAdd(Path fileToAdd) {
if (FileUtil.detectBadFileName(fileToAdd.toString())) {
String newFilename = FileNameCleaner.cleanFileName(fileToAdd.getFileName().toString());

boolean correctButtonPressed = dialogService.showConfirmationDialogAndWait(Localization.lang("File \"%0\" cannot be added!", fileToAdd.getFileName()),
Localization.lang("Illegal characters in the file name detected.\nFile will be renamed to \"%0\" and added.", newFilename),
Localization.lang("Rename and add"));

if (correctButtonPressed) {
Path correctPath = fileToAdd.resolveSibling(newFilename);
try {
Files.move(fileToAdd, correctPath);
link.set(relativize(correctPath));
filePreferences.setWorkingDirectory(correctPath);
setExternalFileTypeByExtension(link.getValueSafe());
} catch (IOException ex) {
LOGGER.error("Error moving file", ex);
dialogService.showErrorDialogAndWait(ex);
}
}
}
}

public void setValues(LinkedFile linkedFile) {
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/org/jabref/logic/util/io/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -576,13 +576,10 @@ public static String shortenFileName(String fileName, Integer maxLength) {
numCharsBeforeEllipsis = Math.min(numCharsBeforeEllipsis, name.length());
numCharsAfterEllipsis = Math.min(numCharsAfterEllipsis, name.length() - numCharsBeforeEllipsis);

StringBuilder result = new StringBuilder();
result.append(name, 0, numCharsBeforeEllipsis)
.append(ELLIPSIS)
.append(name.substring(name.length() - numCharsAfterEllipsis))
.append(extension);

return result.toString();
return name.substring(0, numCharsBeforeEllipsis) +
ELLIPSIS +
name.substring(name.length() - numCharsAfterEllipsis) +
extension;
}

public static boolean isCharLegal(char c) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.jabref.gui.linkedfile;

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

import javafx.collections.FXCollections;

import org.jabref.gui.DialogService;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.frame.ExternalApplicationsPreferences;
import org.jabref.gui.preferences.GuiPreferences;
import org.jabref.logic.FilePreferences;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.LinkedFile;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Answers;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class LinkedFileEditDialogViewModelTest {
private final GuiPreferences preferences = mock(GuiPreferences.class, Answers.RETURNS_DEEP_STUBS);
private final FilePreferences filePreferences = mock(FilePreferences.class, Answers.RETURNS_DEEP_STUBS);
private final BibDatabaseContext bibDatabaseContext = mock(BibDatabaseContext.class);
private final DialogService dialogService = mock(DialogService.class);
private final ExternalApplicationsPreferences externalApplicationsPreferences = mock(ExternalApplicationsPreferences.class);

@BeforeEach
void setup() {
when(externalApplicationsPreferences.getExternalFileTypes()).thenReturn(FXCollections.observableSet(new TreeSet<>(ExternalFileTypes.getDefaultExternalFileTypes())));
when(preferences.getExternalApplicationsPreferences()).thenReturn(externalApplicationsPreferences);
when(preferences.getFilePreferences()).thenReturn(filePreferences);
}

@Test
void badFilenameCharWillBeReplacedByUnderscore(@TempDir Path tempDir) throws Exception {

Path invalidFile = tempDir.resolve("?invalid.pdf");
Files.createFile(invalidFile);
when(dialogService.showConfirmationDialogAndWait(any(), any(), any())).thenReturn(true);

LinkedFileEditDialogViewModel viewModel = new LinkedFileEditDialogViewModel(new LinkedFile("", "", ""), bibDatabaseContext, dialogService, externalApplicationsPreferences, filePreferences);

viewModel.checkForBadFileNameAndAdd(invalidFile);

LinkedFile expectedFile = new LinkedFile("", tempDir.resolve("_invalid.pdf").toString(), "PDF");
assertEquals(expectedFile, viewModel.getNewLinkedFile());
}
}

0 comments on commit 4f8ece3

Please sign in to comment.