Skip to content

Commit

Permalink
Fix downloading pdf produces html as extension (#4965)
Browse files Browse the repository at this point in the history 
* Fix downloading pdf produces html as extension

If we already have a filetype we use that instead of relying on the autodetection


Fixes #4913

* add relativze if not an URL

* Create  Test for download pdf document
heavy mocking and refactoring of ExternalFileType

TODO : Cleanup

* refactor and simply test
fix cehckstyle
fail test on exception
  • Loading branch information
@Siedlerchr
Siedlerchr authored May 18, 2019
1 parent cbd3fdf commit 102ed25
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 61 deletions.
5 changes: 3 additions & 2 deletions src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.jabref.gui.BasePanel;
import org.jabref.gui.DialogService;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.fieldeditors.LinkedFileViewModel;
import org.jabref.gui.fieldeditors.LinkedFilesEditorViewModel;
import org.jabref.gui.util.BackgroundTask;
Expand Down Expand Up @@ -136,15 +137,15 @@ private void addLinkedFileFromURL(URL url, BibEntry entry, Path targetDirectory)
basePanel.getBibDatabaseContext(),
Globals.TASK_EXECUTOR,
dialogService,
JabRefPreferences.getInstance());
JabRefPreferences.getInstance(), ExternalFileTypes.getInstance());

try {
URLDownload urlDownload = new URLDownload(newLinkedFile.getLink());
BackgroundTask<Path> downloadTask = onlineFile.prepareDownloadTask(targetDirectory, urlDownload);
downloadTask.onSuccess(destination -> {
LinkedFile downloadedFile = LinkedFilesEditorViewModel.fromFile(
destination,
basePanel.getBibDatabaseContext().getFileDirectoriesAsPaths(JabRefPreferences.getInstance().getFilePreferences()));
basePanel.getBibDatabaseContext().getFileDirectoriesAsPaths(JabRefPreferences.getInstance().getFilePreferences()), ExternalFileTypes.getInstance());
entry.addFile(downloadedFile);
dialogService.notify(Localization.lang("Finished downloading full text document for entry %0.",
entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
Expand Down
13 changes: 8 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,15 @@ public class LinkedFileViewModel extends AbstractViewModel {
private final FilePreferences filePreferences;
private final XmpPreferences xmpPreferences;
private final LinkedFileHandler linkedFileHandler;
private final ExternalFileTypes externalFileTypes;

public LinkedFileViewModel(LinkedFile linkedFile,
BibEntry entry,
BibDatabaseContext databaseContext,
TaskExecutor taskExecutor,
DialogService dialogService,
JabRefPreferences preferences) {
JabRefPreferences preferences,
ExternalFileTypes externalFileTypes) {

this.linkedFile = linkedFile;
this.filePreferences = preferences.getFilePreferences();
Expand All @@ -81,6 +83,7 @@ public LinkedFileViewModel(LinkedFile linkedFile,
this.entry = entry;
this.dialogService = dialogService;
this.taskExecutor = taskExecutor;
this.externalFileTypes = externalFileTypes;

xmpPreferences = preferences.getXMPPreferences();
downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(1)));
Expand Down Expand Up @@ -402,7 +405,7 @@ public void download() {
URLDownload urlDownload = new URLDownload(linkedFile.getLink());
BackgroundTask<Path> downloadTask = prepareDownloadTask(targetDirectory.get(), urlDownload);
downloadTask.onSuccess(destination -> {
LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences));
LinkedFile newLinkedFile = LinkedFilesEditorViewModel.fromFile(destination, databaseContext.getFileDirectoriesAsPaths(filePreferences), externalFileTypes);
linkedFile.setLink(newLinkedFile.getLink());
linkedFile.setFileType(newLinkedFile.getFileType());
});
Expand All @@ -420,7 +423,7 @@ public BackgroundTask<Path> prepareDownloadTask(Path targetDirectory, URLDownloa
String suggestedTypeName = suggestedType.map(ExternalFileType::getName).orElse("");
linkedFile.setFileType(suggestedTypeName);

String suggestedName = linkedFileHandler.getSuggestedFileName();
String suggestedName = linkedFileHandler.getSuggestedFileName(suggestedTypeName);
return targetDirectory.resolve(suggestedName);
})
.then(destination -> new FileDownloadTask(urlDownload.getSource(), destination))
Expand All @@ -443,15 +446,15 @@ private Optional<ExternalFileType> inferFileTypeFromMimeType(URLDownload urlDown

if (mimeType != null) {
LOGGER.debug("MIME Type suggested: " + mimeType);
return ExternalFileTypes.getInstance().getExternalFileTypeByMimeType(mimeType);
return externalFileTypes.getExternalFileTypeByMimeType(mimeType);
} else {
return Optional.empty();
}
}

private Optional<ExternalFileType> inferFileTypeFromURL(String url) {
return URLUtil.getSuffix(url)
.flatMap(extension -> ExternalFileTypes.getInstance().getExternalFileTypeByExt(extension));
.flatMap(extension -> externalFileTypes.getExternalFileTypeByExt(extension));
}

public LinkedFile getFile() {
Expand Down
21 changes: 11 additions & 10 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,22 @@ public class LinkedFilesEditorViewModel extends AbstractEditorViewModel {
private final BibDatabaseContext databaseContext;
private final TaskExecutor taskExecutor;
private final JabRefPreferences preferences;
private final ExternalFileTypes externalFileTypes = ExternalFileTypes.getInstance();

public LinkedFilesEditorViewModel(String fieldName, AutoCompleteSuggestionProvider<?> suggestionProvider,
DialogService dialogService,
BibDatabaseContext databaseContext,
TaskExecutor taskExecutor,
FieldCheckers fieldCheckers,
JabRefPreferences preferences) {

super(fieldName, suggestionProvider, fieldCheckers);

this.dialogService = dialogService;
this.databaseContext = databaseContext;
this.taskExecutor = taskExecutor;
this.preferences = preferences;


BindingsHelper.bindContentBidirectional(
files,
text,
Expand All @@ -86,9 +87,9 @@ private static String getStringRepresentation(List<LinkedFileViewModel> files) {
*
* TODO: Move this method to {@link LinkedFile} as soon as {@link CustomExternalFileType} lives in model.
*/
public static LinkedFile fromFile(Path file, List<Path> fileDirectories) {
public static LinkedFile fromFile(Path file, List<Path> fileDirectories, ExternalFileTypes externalFileTypesFile) {
String fileExtension = FileHelper.getFileExtension(file).orElse("");
ExternalFileType suggestedFileType = ExternalFileTypes.getInstance()
ExternalFileType suggestedFileType = externalFileTypesFile
.getExternalFileTypeByExt(fileExtension)
.orElse(new UnknownExternalFileType(fileExtension));
Path relativePath = FileUtil.relativize(file, fileDirectories);
Expand All @@ -98,8 +99,8 @@ public static LinkedFile fromFile(Path file, List<Path> fileDirectories) {
public LinkedFileViewModel fromFile(Path file) {
List<Path> fileDirectories = databaseContext.getFileDirectoriesAsPaths(preferences.getFilePreferences());

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

}

Expand All @@ -113,7 +114,7 @@ public BooleanProperty fulltextLookupInProgressProperty() {

private List<LinkedFileViewModel> parseToFileViewModel(String stringValue) {
return FileFieldParser.parse(stringValue).stream()
.map(linkedFile -> new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences))
.map(linkedFile -> new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes))
.collect(Collectors.toList());
}

Expand All @@ -135,8 +136,8 @@ public void addNewFile() {

List<Path> fileDirectories = databaseContext.getFileDirectoriesAsPaths(preferences.getFilePreferences());
dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(newFile -> {
LinkedFile newLinkedFile = fromFile(newFile, fileDirectories);
files.add(new LinkedFileViewModel(newLinkedFile, entry, databaseContext, taskExecutor, dialogService, preferences));
LinkedFile newLinkedFile = fromFile(newFile, fileDirectories, externalFileTypes);
files.add(new LinkedFileViewModel(newLinkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes));
});
}

Expand All @@ -162,7 +163,7 @@ private List<LinkedFileViewModel> findAssociatedNotLinkedFiles(BibEntry entry) {
try {
List<LinkedFile> linkedFiles = util.findAssociatedNotLinkedFiles(entry);
for (LinkedFile linkedFile : linkedFiles) {
LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences);
LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes);
newLinkedFile.markAsAutomaticallyFound();
result.add(newLinkedFile);
}
Expand Down Expand Up @@ -205,7 +206,7 @@ public void addFromURL() {
}

private void addFromURL(URL url) {
LinkedFileViewModel onlineFile = new LinkedFileViewModel(new LinkedFile(url, ""), entry, databaseContext, taskExecutor, dialogService, preferences);
LinkedFileViewModel onlineFile = new LinkedFileViewModel(new LinkedFile(url, ""), entry, databaseContext, taskExecutor, dialogService, preferences, externalFileTypes);
files.add(onlineFile);
onlineFile.download();
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jabref/gui/filelist/AttachFileAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.jabref.gui.BasePanel;
import org.jabref.gui.DialogService;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.fieldeditors.LinkedFilesEditorViewModel;
import org.jabref.gui.undo.UndoableFieldChange;
import org.jabref.gui.util.FileDialogConfiguration;
Expand Down Expand Up @@ -38,7 +39,7 @@ public void execute() {

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(newFile -> {

LinkedFile linkedFile = LinkedFilesEditorViewModel.fromFile(newFile, panel.getBibDatabaseContext().getFileDirectoriesAsPaths(Globals.prefs.getFilePreferences()));
LinkedFile linkedFile = LinkedFilesEditorViewModel.fromFile(newFile, panel.getBibDatabaseContext().getFileDirectoriesAsPaths(Globals.prefs.getFilePreferences()), ExternalFileTypes.getInstance());

LinkedFileEditDialogView dialog = new LinkedFileEditDialogView(linkedFile);

Expand Down
13 changes: 8 additions & 5 deletions src/main/java/org/jabref/gui/filelist/LinkedFilesEditDialogViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public void openBrowseDialog() {
String fileName = Paths.get(fileText).getFileName().toString();

FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.withInitialDirectory(workingDir)
.withInitialFileName(fileName)
.build();
.withInitialDirectory(workingDir)
.withInitialFileName(fileName)
.build();

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(path -> {
// Store the directory for next time:
Expand All @@ -92,8 +92,11 @@ public void openBrowseDialog() {
public void setValues(LinkedFile linkedFile) {
description.set(linkedFile.getDescription());

Path linkPath = Paths.get(linkedFile.getLink());
link.set(relativize(linkPath));
if (linkedFile.isOnlineLink()) {
link.setValue(linkedFile.getLink()); //Might be an URL
} else {
link.setValue(relativize(Paths.get(linkedFile.getLink())));
}

selectedExternalFileType.setValue(null);

Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class MainTableColumnFactory {
private final CellFactory cellFactory;
private final UndoManager undoManager;
private final DialogService dialogService;


public MainTableColumnFactory(BibDatabaseContext database, ColumnPreferences preferences, ExternalFileTypes externalFileTypes, UndoManager undoManager, DialogService dialogService) {
this.database = Objects.requireNonNull(database);
Expand Down Expand Up @@ -264,7 +265,7 @@ private TableColumn<BibEntryTableViewModel, List<LinkedFile>> createFileColumn()
.withOnMouseClickedEvent((entry, linkedFiles) -> event -> {
if ((event.getButton() == MouseButton.PRIMARY) && (linkedFiles.size() == 1)) {
// Only one linked file -> open directly
LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFiles.get(0), entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs);
LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFiles.get(0), entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs, externalFileTypes);
linkedFileViewModel.open();
}
})
Expand All @@ -287,7 +288,7 @@ private ContextMenu createFileMenu(BibEntryTableViewModel entry, List<LinkedFile
ContextMenu contextMenu = new ContextMenu();

for (LinkedFile linkedFile : linkedFiles) {
LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFile, entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs);
LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel(linkedFile, entry.getEntry(), database, Globals.TASK_EXECUTOR, dialogService, Globals.prefs, externalFileTypes);

MenuItem menuItem = new MenuItem(linkedFileViewModel.getDescriptionAndLink(), linkedFileViewModel.getTypeIcon().getGraphicNode());
menuItem.setOnAction(event -> linkedFileViewModel.open());
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.slf4j.LoggerFactory;

public class LinkedFileHandler {

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

private final BibDatabaseContext databaseContext;
Expand Down Expand Up @@ -84,7 +85,7 @@ public boolean renameToName(String targetFileName) throws IOException {

String expandedOldFilePath = oldFile.get().toString();
boolean pathsDifferOnlyByCase = newPath.toString().equalsIgnoreCase(expandedOldFilePath)
&& !newPath.toString().equals(expandedOldFilePath);
&& !newPath.toString().equals(expandedOldFilePath);

if (Files.exists(newPath) && !pathsDifferOnlyByCase) {
// We do not overwrite files
Expand Down Expand Up @@ -113,9 +114,14 @@ private String relativize(Path path) {
public String getSuggestedFileName() {
String oldFileName = fileEntry.getLink();

String extension = FileHelper.getFileExtension(oldFileName).orElse(fileEntry.getFileType());
return getSuggestedFileName(extension);
}

public String getSuggestedFileName(String extension) {
String targetFileName = FileUtil.createFileNameFromPattern(databaseContext.getDatabase(), entry, filePreferences.getFileNamePattern()).trim()
+ '.'
+ FileHelper.getFileExtension(oldFileName).orElse(fileEntry.getFileType());
+ '.'
+ extension;

// Only create valid file names
return FileUtil.getValidFileName(targetFileName);
Expand Down
Loading

0 comments on commit 102ed25

Please sign in to comment.