diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java new file mode 100644 index 00000000000..0fa1fce83f3 --- /dev/null +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java @@ -0,0 +1,68 @@ +package org.jabref.gui.externalfiles; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.jabref.gui.externalfiletype.ExternalFileType; +import org.jabref.gui.externalfiletype.ExternalFileTypes; +import org.jabref.gui.externalfiletype.UnknownExternalFileType; +import org.jabref.logic.util.io.AutoLinkPreferences; +import org.jabref.logic.util.io.FileFinder; +import org.jabref.logic.util.io.FileFinders; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.LinkedFile; +import org.jabref.model.metadata.FileDirectoryPreferences; +import org.jabref.model.util.FileHelper; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +public class AutoSetFileLinksUtil { + + private static final Log LOGGER = LogFactory.getLog(AutoSetLinks.class); + + public List findassociatedNotLinkedFiles(BibEntry entry, BibDatabaseContext databaseContext, FileDirectoryPreferences fileDirPrefs, AutoLinkPreferences autoLinkPrefs, ExternalFileTypes externalFileTypes) { + List linkedFiles = new ArrayList<>(); + + List dirs = databaseContext.getFileDirectoriesAsPaths(fileDirPrefs); + List extensions = externalFileTypes.getExternalFileTypeSelection().stream().map(ExternalFileType::getExtension).collect(Collectors.toList()); + + // Run the search operation: + FileFinder fileFinder = FileFinders.constructFromConfiguration(autoLinkPrefs); + List result = fileFinder.findAssociatedFiles(entry, dirs, extensions); + + // Iterate over the entries: + + for (Path foundFile : result) { + boolean existingSameFile = entry.getFiles().stream() + .map(file -> file.findIn(dirs)) + .anyMatch(file -> { + try { + return file.isPresent() && Files.isSameFile(file.get(), foundFile); + } catch (IOException e) { + LOGGER.error("Problem with isSameFile", e); + } + return false; + }); + if (!existingSameFile) { + + Optional type = FileHelper.getFileExtension(foundFile) + .map(externalFileTypes::getExternalFileTypeByExt) + .orElse(Optional.of(new UnknownExternalFileType(""))); + + String strType = type.isPresent() ? type.get().getName() : ""; + + LinkedFile linkedFile = new LinkedFile("", foundFile.toString(), strType); + linkedFiles.add(linkedFile); + } + } + + return linkedFiles; + } +} diff --git a/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java b/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java index 4e63fffcb00..b8f15487205 100644 --- a/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java +++ b/src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java @@ -3,16 +3,10 @@ import java.awt.BorderLayout; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import javax.swing.BorderFactory; import javax.swing.JDialog; @@ -24,19 +18,15 @@ import org.jabref.Globals; import org.jabref.gui.externalfiletype.ExternalFileType; import org.jabref.gui.externalfiletype.ExternalFileTypes; -import org.jabref.gui.externalfiletype.UnknownExternalFileType; -import org.jabref.gui.filelist.FileListEntry; -import org.jabref.gui.filelist.FileListTableModel; import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableFieldChange; +import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.logic.l10n.Localization; -import org.jabref.logic.util.io.FileFinder; -import org.jabref.logic.util.io.FileFinders; -import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.FieldName; -import org.jabref.model.util.FileHelper; +import org.jabref.model.entry.FileFieldWriter; +import org.jabref.model.entry.LinkedFile; public class AutoSetLinks { @@ -50,7 +40,7 @@ private AutoSetLinks() { * @param databaseContext the database for which links are set */ public static void autoSetLinks(List entries, BibDatabaseContext databaseContext) { - autoSetLinks(entries, null, null, null, databaseContext, null, null); + autoSetLinks(entries, null, null, databaseContext, null, null); } /** @@ -64,12 +54,7 @@ public static void autoSetLinks(List entries, BibDatabaseContext datab * @param entries A collection of BibEntry objects to find links for. * @param ce A NamedCompound to add UndoEdit elements to. * @param changedEntries MODIFIED, optional. A Set of BibEntry objects to which all modified entries is added. - * This is used for status output and debugging - * @param singleTableModel UGLY HACK. The table model to insert links into. Already existing links are not - * duplicated or removed. This parameter has to be null if entries.count() != 1. The hack has been - * introduced as a bibtexentry does not (yet) support the function getListTableModel() and the - * FileListEntryEditor editor holds an instance of that table model and does not reconstruct it after the - * search has succeeded. + * @param databaseContext The database providing the relevant file directory, if any. * @param callback An ActionListener that is notified (on the event dispatch thread) when the search is finished. * The ActionEvent has id=0 if no new links were added, and id=1 if one or more links were added. This @@ -79,7 +64,7 @@ public static void autoSetLinks(List entries, BibDatabaseContext datab * @return the thread performing the automatically setting */ public static Runnable autoSetLinks(final List entries, final NamedCompound ce, - final Set changedEntries, final FileListTableModel singleTableModel, + final Set changedEntries, final BibDatabaseContext databaseContext, final ActionListener callback, final JDialog diag) { final Collection types = ExternalFileTypes.getInstance().getExternalFileTypeSelection(); if (diag != null) { @@ -95,97 +80,58 @@ public static Runnable autoSetLinks(final List entries, final NamedCom diag.setLocationRelativeTo(diag.getParent()); } - Runnable r = new Runnable() { - - @Override - public void run() { - // determine directories to search in - final List dirs = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences()); - - // determine extensions - final List extensions = types.stream().map(ExternalFileType::getExtension).collect(Collectors.toList()); - - // Run the search operation: - FileFinder fileFinder = FileFinders.constructFromConfiguration(Globals.prefs.getAutoLinkPreferences()); - Map> result = fileFinder.findAssociatedFiles(entries, dirs, extensions); - - boolean foundAny = false; - // Iterate over the entries: - for (Entry> entryFilePair : result.entrySet()) { - FileListTableModel tableModel; - Optional oldVal = entryFilePair.getKey().getField(FieldName.FILE); - if (singleTableModel == null) { - tableModel = new FileListTableModel(); - oldVal.ifPresent(tableModel::setContent); - } else { - assert entries.size() == 1; - tableModel = singleTableModel; + Runnable r = () -> { + boolean foundAny = false; + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); + + for (BibEntry entry : entries) { + + List linkedFiles = util.findassociatedNotLinkedFiles(entry, databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); + + if (ce != null) { + for (LinkedFile linkedFile : linkedFiles) { + // store undo information + String newVal = FileFieldWriter.getStringRepresentation(linkedFile); + + String oldVal = entry.getField(FieldName.FILE).orElse(null); + + UndoableFieldChange fieldChange = new UndoableFieldChange(entry, FieldName.FILE, oldVal, newVal); + ce.addEdit(fieldChange); + + DefaultTaskExecutor.runInJavaFXThread(() -> { + entry.addFile(linkedFile); + }); + foundAny = true; } - List files = entryFilePair.getValue(); - for (Path file : files) { - file = FileUtil.shortenFileName(file, dirs); - boolean alreadyHas = false; - - for (int j = 0; j < tableModel.getRowCount(); j++) { - FileListEntry existingEntry = tableModel.getEntry(j); - if (Paths.get(existingEntry.getLink()).equals(file)) { - alreadyHas = true; - foundAny = true; - break; - } - } - if (!alreadyHas) { - foundAny = true; - Optional type = FileHelper.getFileExtension(file) - .map(extension -> ExternalFileTypes.getInstance().getExternalFileTypeByExt(extension)) - .orElse(Optional.of(new UnknownExternalFileType(""))); - FileListEntry flEntry = new FileListEntry("", file.toString(), type); - tableModel.addEntry(tableModel.getRowCount(), flEntry); - - String newVal = tableModel.getStringRepresentation(); - if (newVal.isEmpty()) { - newVal = null; - } - if (ce != null) { - // store undo information - UndoableFieldChange change = new UndoableFieldChange(entryFilePair.getKey(), - FieldName.FILE, oldVal.orElse(null), newVal); - ce.addEdit(change); - } - // hack: if table model is given, do NOT modify entry - if (singleTableModel == null) { - entryFilePair.getKey().setField(FieldName.FILE, newVal); - } - if (changedEntries != null) { - changedEntries.add(entryFilePair.getKey()); - } - } + + if (changedEntries != null) { + changedEntries.add(entry); } } - // handle callbacks and dialog - // FIXME: The ID signals if action was successful :/ - final int id = foundAny ? 1 : 0; - SwingUtilities.invokeLater(new Runnable() { - - @Override - public void run() { - if (diag != null) { - diag.dispose(); - } - if (callback != null) { - callback.actionPerformed(new ActionEvent(this, id, "")); - } - } - }); } + + final int id = foundAny ? 1 : 0; + SwingUtilities.invokeLater(() -> { + + if (diag != null) { + diag.dispose(); + } + if (callback != null) { + callback.actionPerformed(new ActionEvent(AutoSetLinks.class, id, "")); + } + + }); + }; + SwingUtilities.invokeLater(() -> { // show dialog which will be hidden when the task is done if (diag != null) { diag.setVisible(true); } }); + return r; } @@ -194,8 +140,7 @@ public void run() { * of external file types. The entry itself is not modified. The entry's bibtex key must have been set. * * @param entry The BibEntry to find links for. - * @param singleTableModel The table model to insert links into. Already existing links are not duplicated or - * removed. + * @param databaseContext The database providing the relevant file directory, if any. * @param callback An ActionListener that is notified (on the event dispatch thread) when the search is finished. * The ActionEvent has id=0 if no new links were added, and id=1 if one or more links were added. This @@ -206,9 +151,9 @@ public void run() { * parameter can be null, which means that no progress update will be shown. * @return the runnable able to perform the automatically setting */ - public static Runnable autoSetLinks(final BibEntry entry, final FileListTableModel singleTableModel, + public static Runnable autoSetLinks(final BibEntry entry, final BibDatabaseContext databaseContext, final ActionListener callback, final JDialog diag) { - return autoSetLinks(Collections.singletonList(entry), null, null, singleTableModel, databaseContext, callback, + return autoSetLinks(Collections.singletonList(entry), null, null, databaseContext, callback, diag); } diff --git a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java index 5bf278dcd9f..aaf44053987 100644 --- a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java +++ b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.net.URL; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -14,6 +15,7 @@ import org.jabref.Globals; import org.jabref.gui.BasePanel; import org.jabref.gui.undo.UndoableFieldChange; +import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.gui.worker.AbstractWorker; import org.jabref.logic.importer.FulltextFetchers; import org.jabref.logic.l10n.Localization; @@ -81,9 +83,9 @@ public void update() { BibEntry entry = download.getValue(); Optional result = download.getKey(); if (result.isPresent()) { - List dirs = basePanel.getBibDatabaseContext() - .getFileDirectories(Globals.prefs.getFileDirectoryPreferences()); - if (dirs.isEmpty()) { + Optional dir = basePanel.getBibDatabaseContext().getFirstExistingFileDir(Globals.prefs.getFileDirectoryPreferences()); + + if (!dir.isPresent()) { JOptionPane.showMessageDialog(basePanel.frame(), Localization.lang("Main file directory not set!") + " " + Localization.lang("Preferences") + " -> " + Localization.lang("File"), @@ -94,13 +96,16 @@ public void update() { basePanel.getBibDatabaseContext(), entry); try { def.download(result.get(), file -> { - Optional fieldChange = entry.addFile(file); - if (fieldChange.isPresent()) { - UndoableFieldChange edit = new UndoableFieldChange(entry, FieldName.FILE, - entry.getField(FieldName.FILE).orElse(null), fieldChange.get().getNewValue()); - basePanel.getUndoManager().addEdit(edit); - basePanel.markBaseChanged(); - } + DefaultTaskExecutor.runInJavaFXThread(() -> { + Optional fieldChange = entry.addFile(file); + if (fieldChange.isPresent()) { + UndoableFieldChange edit = new UndoableFieldChange(entry, FieldName.FILE, + entry.getField(FieldName.FILE).orElse(null), fieldChange.get().getNewValue()); + basePanel.getUndoManager().addEdit(edit); + basePanel.markBaseChanged(); + } + }); + }); } catch (IOException e) { LOGGER.warn("Problem downloading file", e); diff --git a/src/main/java/org/jabref/gui/externalfiles/SynchronizeFileField.java b/src/main/java/org/jabref/gui/externalfiles/SynchronizeFileField.java index 2cbda14fcea..7f8a5011fa7 100644 --- a/src/main/java/org/jabref/gui/externalfiles/SynchronizeFileField.java +++ b/src/main/java/org/jabref/gui/externalfiles/SynchronizeFileField.java @@ -120,7 +120,7 @@ public void run() { List entries = new ArrayList<>(sel); // Start the automatically setting process: - Runnable r = AutoSetLinks.autoSetLinks(entries, ce, changedEntries, null, panel.getBibDatabaseContext(), null, null); + Runnable r = AutoSetLinks.autoSetLinks(entries, ce, changedEntries, panel.getBibDatabaseContext(), null, null); JabRefExecutorService.INSTANCE.executeAndWait(r); } progress += sel.size() * weightAutoSet; diff --git a/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java b/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java index 8b9d9193204..702b47048bd 100644 --- a/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java +++ b/src/main/java/org/jabref/gui/externalfiletype/ExternalFileTypes.java @@ -17,7 +17,8 @@ import org.jabref.model.util.FileHelper; import org.jabref.preferences.JabRefPreferences; -public final class ExternalFileTypes { +//Do not make this class final, as it otherwise can't be mocked for tests +public class ExternalFileTypes { // This String is used in the encoded list in prefs of external file type // modifications, in order to indicate a removed default file type: @@ -115,10 +116,9 @@ public Set getExternalFileTypeSelection() { * @return The ExternalFileType registered, or null if none. */ public Optional getExternalFileTypeByName(String name) { - for (ExternalFileType type : externalFileTypes) { - if (type.getName().equals(name)) { - return Optional.of(type); - } + Optional externalFileType = externalFileTypes.stream().filter(type -> type.getExtension().equals(name)).findFirst(); + if (externalFileType.isPresent()) { + return externalFileType; } // Return an instance that signifies an unknown file type: return Optional.of(new UnknownExternalFileType(name)); @@ -131,12 +131,7 @@ public Optional getExternalFileTypeByName(String name) { * @return The ExternalFileType registered, or null if none. */ public Optional getExternalFileTypeByExt(String extension) { - for (ExternalFileType type : externalFileTypes) { - if (type.getExtension().equalsIgnoreCase(extension)) { - return Optional.of(type); - } - } - return Optional.empty(); + return externalFileTypes.stream().filter(type -> type.getExtension().equalsIgnoreCase(extension)).findFirst(); } /** @@ -146,27 +141,7 @@ public Optional getExternalFileTypeByExt(String extension) { * @return true if an ExternalFileType with the extension exists, false otherwise */ public boolean isExternalFileTypeByExt(String extension) { - for (ExternalFileType type : externalFileTypes) { - if (type.getExtension().equalsIgnoreCase(extension)) { - return true; - } - } - return false; - } - - /** - * Look up the external file type name registered for this extension, if any. - * - * @param extension The file extension. - * @return The name of the ExternalFileType registered, or null if none. - */ - public String getExternalFileTypeNameByExt(String extension) { - for (ExternalFileType type : externalFileTypes) { - if (type.getExtension().equalsIgnoreCase(extension)) { - return type.getName(); - } - } - return ""; + return externalFileTypes.stream().anyMatch(type -> type.getExtension().equalsIgnoreCase(extension)); } /** diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java index 4ce91d23bf7..5860b731501 100644 --- a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java +++ b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java @@ -20,6 +20,7 @@ import org.jabref.Globals; import org.jabref.gui.DialogService; import org.jabref.gui.autocompleter.AutoCompleteSuggestionProvider; +import org.jabref.gui.externalfiles.AutoSetFileLinksUtil; import org.jabref.gui.externalfiles.DownloadExternalFile; import org.jabref.gui.externalfiles.FileDownloadTask; import org.jabref.gui.externalfiletype.ExternalFileType; @@ -34,8 +35,6 @@ import org.jabref.logic.l10n.Localization; import org.jabref.logic.net.URLDownload; import org.jabref.logic.util.OS; -import org.jabref.logic.util.io.FileFinder; -import org.jabref.logic.util.io.FileFinders; import org.jabref.logic.util.io.FileUtil; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; @@ -157,24 +156,17 @@ public void bindToEntry(BibEntry entry) { * Find files that are probably associated to the given entry but not yet linked. */ private List findAssociatedNotLinkedFiles(BibEntry entry) { - final List dirs = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences()); - final List extensions = ExternalFileTypes.getInstance().getExternalFileTypeSelection().stream().map(ExternalFileType::getExtension).collect(Collectors.toList()); + List result = new ArrayList<>(); - // Run the search operation: - FileFinder fileFinder = FileFinders.constructFromConfiguration(Globals.prefs.getAutoLinkPreferences()); - List newFiles = fileFinder.findAssociatedFiles(entry, dirs, extensions); + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); + List linkedFiles = util.findassociatedNotLinkedFiles(entry, databaseContext, Globals.prefs.getFileDirectoryPreferences(), Globals.prefs.getAutoLinkPreferences(), ExternalFileTypes.getInstance()); - List result = new ArrayList<>(); - for (Path newFile : newFiles) { - boolean alreadyLinked = files.get().stream() - .map(file -> file.findIn(dirs)) - .anyMatch(file -> file.isPresent() && file.get().equals(newFile)); - if (!alreadyLinked) { - LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(fromFile(newFile, dirs), entry, databaseContext); - newLinkedFile.markAsAutomaticallyFound(); - result.add(newLinkedFile); - } + for (LinkedFile linkedFile : linkedFiles) { + LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(linkedFile, entry, databaseContext); + newLinkedFile.markAsAutomaticallyFound(); + result.add(newLinkedFile); } + return result; } diff --git a/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java b/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java new file mode 100644 index 00000000000..8bf55431ced --- /dev/null +++ b/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java @@ -0,0 +1,55 @@ +package org.jabref.gui.externalfiles; + +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; +import java.util.TreeSet; + +import org.jabref.gui.externalfiletype.ExternalFileTypes; +import org.jabref.logic.util.io.AutoLinkPreferences; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.LinkedFile; +import org.jabref.model.metadata.FileDirectoryPreferences; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class AutoSetFileLinksUtilTest { + + private final FileDirectoryPreferences fileDirPrefs = mock(FileDirectoryPreferences.class); + private final AutoLinkPreferences autoLinkPrefs = new AutoLinkPreferences(false, "", true, ';'); + private final BibDatabaseContext databaseContext = mock(BibDatabaseContext.class); + private final ExternalFileTypes externalFileTypes = mock(ExternalFileTypes.class); + private final BibEntry entry = new BibEntry("article"); + private Path file; + + @Rule public TemporaryFolder folder = new TemporaryFolder(); + + @Before + public void setUp() throws Exception { + entry.setCiteKey("CiteKey"); + file = folder.newFile("CiteKey.pdf").toPath(); + when(databaseContext.getFileDirectoriesAsPaths(any())).thenReturn(Collections.singletonList(folder.getRoot().toPath())); + when(externalFileTypes.getExternalFileTypeSelection()).thenReturn(new TreeSet<>(externalFileTypes.getDefaultExternalFileTypes())); + + } + + @Test + public void test() { + //Due to mocking the externalFileType class, the file extension will not be found + List expected = Collections.singletonList(new LinkedFile("", file.toString(), "")); + + AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(); + List actual = util.findassociatedNotLinkedFiles(entry, databaseContext, fileDirPrefs, autoLinkPrefs, externalFileTypes); + assertEquals(expected, actual); + } + +}