Skip to content

Commit

Permalink
Refactorings (#3370)
Browse files Browse the repository at this point in the history 
* Rework AutoSetFileLinks

* Only add first found file

* Improve magic AutoFileLinking in entry editor by using same logic as manually triggering it

* Don't use paths.get

* add changelog entry

* replcaed some occurrences of vector with arrayList

* fix checkstyle
  • Loading branch information
@Siedlerchr
Siedlerchr authored Oct 29, 2017
1 parent 2de829e commit 4178789
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 115 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where JabRef would freeze when trying to replace the original entry after a merge with new information from identifiers like DOI/ISBN etc. [3294](https://github.com/JabRef/jabref/issues/3294)
- We fixed an issue where JabRef would not show the translated content at some points, although there existed a translation
- We fixed an issue where editing in the source tab would override content of other entries [#3352](https://github.com/JabRef/jabref/issues/3352#issue-268580818)
### Removed
- We fixed several issues with the automatic linking of files in the entry editor where files were not found or not correctly saved in the bibtex source [#3346](https://github.com/JabRef/jabref/issues/3346)

### Removed



Expand Down
15 changes: 3 additions & 12 deletions src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.Vector;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.swing.AbstractAction;
Expand Down Expand Up @@ -973,11 +972,7 @@ private void createFileTypesCombobox() {

List<FileFilter> fileFilterList = creatorManager.getFileFilterList();

Vector<FileFilter> vector = new Vector<>();
for (FileFilter fileFilter : fileFilterList) {
vector.add(fileFilter);
}
comboBoxFileTypeSelection = new JComboBox<>(vector);
comboBoxFileTypeSelection = new JComboBox<>(fileFilterList.toArray(new FileFilter[fileFilterList.size()]));

comboBoxFileTypeSelection.setRenderer(new DefaultListCellRenderer() {

Expand Down Expand Up @@ -1009,16 +1004,15 @@ private void createEntryTypesCombobox() {

Iterator<EntryType> iterator = EntryTypes
.getAllValues(frame.getCurrentBasePanel().getBibDatabaseContext().getMode()).iterator();
Vector<BibtexEntryTypeWrapper> list = new Vector<>();
List<BibtexEntryTypeWrapper> list = new ArrayList<>();
list.add(
new BibtexEntryTypeWrapper(null));
while (iterator.hasNext()) {
list.add(new BibtexEntryTypeWrapper(iterator.next()));
}
comboBoxEntryTypeSelection = new JComboBox<>(list);
comboBoxEntryTypeSelection = new JComboBox<>(list.toArray(new BibtexEntryTypeWrapper[list.size()]));
}


/**
* Wrapper for displaying the Type {@link BibtexEntryType} in a Combobox.
*
Expand All @@ -1030,7 +1024,6 @@ private static class BibtexEntryTypeWrapper {

private final EntryType entryType;


BibtexEntryTypeWrapper(EntryType bibtexType) {
this.entryType = bibtexType;
}
Expand All @@ -1053,7 +1046,6 @@ public static class CheckableTreeNode extends DefaultMutableTreeNode {
private boolean isSelected;
private final JCheckBox checkbox;


public CheckableTreeNode(Object userObject) {
super(userObject);
checkbox = new JCheckBox();
Expand Down Expand Up @@ -1134,7 +1126,6 @@ public static class FileNodeWrapper {
public final File file;
public final int fileCount;


public FileNodeWrapper(File aFile) {
this(aFile, 0);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/actions/AutoLinkFilesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void actionPerformed(ActionEvent event) {
}
JDialog diag = new JDialog(JabRefGUI.getMainFrame(), true);
final NamedCompound nc = new NamedCompound(Localization.lang("Automatically set file links"));
Runnable runnable = AutoSetLinks.autoSetLinks(entries, nc, null, null,
Runnable runnable = AutoSetLinks.autoSetLinks(entries, nc, null,
JabRefGUI.getMainFrame().getCurrentBasePanel().getBibDatabaseContext(), e -> {
if (e.getID() > 0) {
// entry has been updated in Util.autoSetLinks, only treat nc and status message
Expand Down
168 changes: 82 additions & 86 deletions src/main/java/org/jabref/gui/externalfiles/AutoSetLinks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
Expand All @@ -25,21 +26,26 @@
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.entry.FileFieldWriter;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.util.FileHelper;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

public class AutoSetLinks {

private static final Log LOGGER = LogFactory.getLog(AutoSetLinks.class);

private AutoSetLinks() {
}

Expand All @@ -50,7 +56,7 @@ private AutoSetLinks() {
* @param databaseContext the database for which links are set
*/
public static void autoSetLinks(List<BibEntry> entries, BibDatabaseContext databaseContext) {
autoSetLinks(entries, null, null, null, databaseContext, null, null);
autoSetLinks(entries, null, null, databaseContext, null, null);
}

/**
Expand Down Expand Up @@ -79,7 +85,7 @@ public static void autoSetLinks(List<BibEntry> entries, BibDatabaseContext datab
* @return the thread performing the automatically setting
*/
public static Runnable autoSetLinks(final List<BibEntry> entries, final NamedCompound ce,
final Set<BibEntry> changedEntries, final FileListTableModel singleTableModel,
final Set<BibEntry> changedEntries,
final BibDatabaseContext databaseContext, final ActionListener callback, final JDialog diag) {
final Collection<ExternalFileType> types = ExternalFileTypes.getInstance().getExternalFileTypeSelection();
if (diag != null) {
Expand All @@ -95,97 +101,87 @@ public static Runnable autoSetLinks(final List<BibEntry> entries, final NamedCom
diag.setLocationRelativeTo(diag.getParent());
}

Runnable r = new Runnable() {

@Override
public void run() {
// determine directories to search in
final List<Path> dirs = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences());

// determine extensions
final List<String> extensions = types.stream().map(ExternalFileType::getExtension).collect(Collectors.toList());

// Run the search operation:
FileFinder fileFinder = FileFinders.constructFromConfiguration(Globals.prefs.getAutoLinkPreferences());
Map<BibEntry, List<Path>> result = fileFinder.findAssociatedFiles(entries, dirs, extensions);

boolean foundAny = false;
// Iterate over the entries:
for (Entry<BibEntry, List<Path>> entryFilePair : result.entrySet()) {
FileListTableModel tableModel;
Optional<String> oldVal = entryFilePair.getKey().getField(FieldName.FILE);
if (singleTableModel == null) {
tableModel = new FileListTableModel();
oldVal.ifPresent(tableModel::setContent);
} else {
assert entries.size() == 1;
tableModel = singleTableModel;
}
List<Path> 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;
}
Runnable r = () -> {

// determine directories to search in
final List<Path> dirs = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences());

// determine extensions
final List<String> extensions = types.stream().map(ExternalFileType::getExtension).collect(Collectors.toList());

// Run the search operation:
FileFinder fileFinder = FileFinders.constructFromConfiguration(Globals.prefs.getAutoLinkPreferences());
Map<BibEntry, List<Path>> result = fileFinder.findAssociatedFiles(entries, dirs, extensions);

boolean foundAny = false;
// Iterate over the entries:
for (Entry<BibEntry, List<Path>> entryFilePair : result.entrySet()) {
Optional<String> oldVal = entryFilePair.getKey().getField(FieldName.FILE);

for (Path foundFile : entryFilePair.getValue()) {
boolean existingSameFile = entryFilePair.getKey().getFiles().stream()
.map(file -> file.findIn(dirs))
.anyMatch(file -> {
try {
return file.isPresent() && Files.isSameFile(file.get(), foundFile);
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
return false;
});
if (!existingSameFile) {

foundAny = true;
Optional<ExternalFileType> type = FileHelper.getFileExtension(foundFile)
.map(extension -> ExternalFileTypes.getInstance().getExternalFileTypeByExt(extension))
.orElse(Optional.of(new UnknownExternalFileType("")));

String strType = type.isPresent() ? type.get().getName() : "";
LinkedFile linkedFile = new LinkedFile("", foundFile.toString(), strType);

DefaultTaskExecutor.runInJavaFXThread(() -> {
entryFilePair.getKey().addFile(linkedFile);
});

String newVal = FileFieldWriter.getStringRepresentation(linkedFile);
if (ce != null) {
// store undo information
UndoableFieldChange change = new UndoableFieldChange(entryFilePair.getKey(),
FieldName.FILE, oldVal.orElse(null), newVal);
ce.addEdit(change);
}
if (!alreadyHas) {
foundAny = true;
Optional<ExternalFileType> 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(entryFilePair.getKey());
}
break;
}
}

// 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;
}

Expand All @@ -206,9 +202,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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/externalfiles/SynchronizeFileField.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void run() {
List<BibEntry> 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;
Expand Down
22 changes: 17 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
Expand Down Expand Up @@ -165,14 +166,25 @@ private List<LinkedFileViewModel> findAssociatedNotLinkedFiles(BibEntry entry) {
List<Path> newFiles = fileFinder.findAssociatedFiles(entry, dirs, extensions);

List<LinkedFileViewModel> result = new ArrayList<>();
for (Path newFile : newFiles) {
boolean alreadyLinked = files.get().stream()
for (Path foundFile : newFiles) {

boolean existingSameFile = 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);
.anyMatch(file -> {
try {
return file.isPresent() && Files.isSameFile(file.get(), foundFile);
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
return false;
});

if (!existingSameFile) {
LinkedFileViewModel newLinkedFile = new LinkedFileViewModel(fromFile(foundFile, dirs), entry, databaseContext);
newLinkedFile.markAsAutomaticallyFound();
result.add(newLinkedFile);
break; //only add first file, if it exists multiple times
}
}
return result;
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/org/jabref/gui/groups/UndoableModifySubtree.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.jabref.gui.groups;

import java.util.ArrayList;
import java.util.List;
import java.util.Vector;

import org.jabref.gui.undo.AbstractUndoableJabRefEdit;
import org.jabref.model.groups.GroupTreeNode;
Expand All @@ -14,10 +14,9 @@ public class UndoableModifySubtree extends AbstractUndoableJabRefEdit {
/** The path to the global groups root node */
private final List<Integer> m_subtreeRootPath;
/** This holds the new subtree (the root's modified children) to allow redo. */
private final List<GroupTreeNode> m_modifiedSubtree = new Vector<>();
private final List<GroupTreeNode> m_modifiedSubtree = new ArrayList<>();
private final String m_name;


/**
*
* @param subtree
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/importer/ImportInspectionDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,7 @@ public void actionPerformed(ActionEvent actionEvent) {
// links:
JDialog diag = new JDialog(ImportInspectionDialog.this, true);
JabRefExecutorService.INSTANCE
.execute(AutoSetLinks.autoSetLinks(entry, localModel, bibDatabaseContext, e -> {
.execute(AutoSetLinks.autoSetLinks(entry, bibDatabaseContext, e -> {
if (e.getID() > 0) {

entries.getReadWriteLock().writeLock().lock();
Expand Down
Loading

0 comments on commit 4178789

Please sign in to comment.