ActionHelper to test for present file (#6151)

* Added ActionHelper to test for present file * Added Validation * Refactored Validation * Refactored unsuccessfully ActionHelper * Fixed issues about opening an online link and applied some suggestions of IntelliJ * Fixed recalculation of selected entry
JabRef · Apr 17, 2020 · 07cb5b8 · 07cb5b8
1 parent accd387
commit 07cb5b8
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 32 deletions.
diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -831,7 +831,7 @@ private MenuBar createMenu() {
 
                     new SeparatorMenuItem(),
 
-                    factory.createMenuItem(StandardActions.SHOW_PDF_VIEWER, new ShowDocumentViewerAction()),
+                    factory.createMenuItem(StandardActions.SHOW_PDF_VIEWER, new ShowDocumentViewerAction(stateManager, prefs)),
                     factory.createMenuItem(StandardActions.EDIT_ENTRY, new OpenEntryEditorAction(this, stateManager)),
                     factory.createMenuItem(StandardActions.OPEN_CONSOLE, new OpenConsoleAction(stateManager))
             );

diff --git a/src/main/java/org/jabref/gui/actions/ActionHelper.java b/src/main/java/org/jabref/gui/actions/ActionHelper.java
@@ -1,14 +1,20 @@
 package org.jabref.gui.actions;
 
+import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import javafx.beans.binding.Bindings;
 import javafx.beans.binding.BooleanExpression;
 
 import org.jabref.gui.StateManager;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.LinkedFile;
 import org.jabref.model.entry.field.Field;
+import org.jabref.model.entry.field.StandardField;
+import org.jabref.model.util.FileHelper;
+import org.jabref.preferences.PreferencesService;
 
 public class ActionHelper {
     public static BooleanExpression needsDatabase(StateManager stateManager) {
@@ -36,4 +42,20 @@ public static BooleanExpression isAnyFieldSetForSelectedEntry(List<Field> fields
                 entry.getFieldsObservable(),
                 stateManager.getSelectedEntries());
     }
+
+    public static BooleanExpression isFilePresentForSelectedEntry(StateManager stateManager, PreferencesService preferencesService) {
+        return Bindings.createBooleanBinding(() -> {
+                    List<LinkedFile> files = stateManager.getSelectedEntries().get(0).getFiles();
+                    if ((files.size() > 0) && stateManager.getActiveDatabase().isPresent()) {
+                        Optional<Path> filename = FileHelper.expandFilename(
+                                stateManager.getActiveDatabase().get(),
+                                files.get(0).getLink(),
+                                preferencesService.getFilePreferences());
+                        return filename.isPresent();
+                    } else {
+                        return false;
+                    }
+                }, stateManager.getSelectedEntries(),
+                stateManager.getSelectedEntries().get(0).getFieldBinding(StandardField.FILE));
+    }
 }
diff --git a/src/main/java/org/jabref/gui/documentviewer/ShowDocumentViewerAction.java b/src/main/java/org/jabref/gui/documentviewer/ShowDocumentViewerAction.java
@@ -1,12 +1,18 @@
 package org.jabref.gui.documentviewer;
 
+import org.jabref.gui.StateManager;
+import org.jabref.gui.actions.ActionHelper;
 import org.jabref.gui.actions.SimpleCommand;
+import org.jabref.preferences.PreferencesService;
 
 public class ShowDocumentViewerAction extends SimpleCommand {
 
+    public ShowDocumentViewerAction(StateManager stateManager, PreferencesService preferences) {
+        this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferences));
+    }
+
     @Override
     public void execute() {
         new DocumentViewerView().show();
     }
-
 }
diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.css b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.css
@@ -86,6 +86,10 @@
     -fx-background-color: -jr-error;
 }
 
+.list-cell:invalid {
+    -fx-background-color: -jr-warn;
+}
+
 .code-area .context-menu {
     -fx-font-family: sans-serif;
 }

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -46,8 +46,13 @@
 import org.jabref.model.entry.LinkedFile;
 import org.jabref.model.metadata.FilePreferences;
 import org.jabref.model.strings.StringUtil;
+import org.jabref.model.util.FileHelper;
 import org.jabref.model.util.OptionalUtil;
 
+import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
+import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
+import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -69,6 +74,8 @@ public class LinkedFileViewModel extends AbstractViewModel {
     private final LinkedFileHandler linkedFileHandler;
     private final ExternalFileTypes externalFileTypes;
 
+    private final Validator fileExistsValidator;
+
     public LinkedFileViewModel(LinkedFile linkedFile,
                                BibEntry entry,
                                BibDatabaseContext databaseContext,
@@ -88,6 +95,18 @@ public LinkedFileViewModel(LinkedFile linkedFile,
         this.externalFileTypes = externalFileTypes;
         this.xmpPreferences = xmpPreferences;
 
+        fileExistsValidator = new FunctionBasedValidator<>(
+                linkedFile.linkProperty(),
+                link -> {
+                    if (linkedFile.isOnlineLink()) {
+                        return true;
+                    } else {
+                        Optional<Path> path = FileHelper.expandFilename(databaseContext, link, filePreferences);
+                        return path.isPresent() && Files.exists(path.get());
+                    }
+                },
+                ValidationMessage.warning(Localization.lang("Could not find file '%0'.", linkedFile.getLink())));
+
         downloadOngoing.bind(downloadProgress.greaterThanOrEqualTo(0).and(downloadProgress.lessThan(1)));
         canWriteXMPMetadata.setValue(!linkedFile.isOnlineLink() && linkedFile.getFileType().equalsIgnoreCase("pdf"));
     }
@@ -172,24 +191,19 @@ public void open() {
 
     public void openFolder() {
         try {
-            Path path = null;
-            // absolute path
-            if (Paths.get(linkedFile.getLink()).isAbsolute()) {
-                path = Paths.get(linkedFile.getLink());
-            } else {
-                // relative to file folder
-                for (Path folder : databaseContext.getFileDirectoriesAsPaths(filePreferences)) {
-                    Path file = folder.resolve(linkedFile.getLink());
-                    if (Files.exists(file)) {
-                        path = file;
-                        break;
-                    }
+            if (!linkedFile.isOnlineLink()) {
+                Optional<Path> resolvedPath = FileHelper.expandFilename(
+                        databaseContext,
+                        linkedFile.getLink(),
+                        filePreferences);
+
+                if (resolvedPath.isPresent()) {
+                    JabRefDesktop.openFolderAndSelectFile(resolvedPath.get());
+                } else {
+                    dialogService.showErrorDialogAndWait(Localization.lang("File not found"));
                 }
-            }
-            if (path != null) {
-                JabRefDesktop.openFolderAndSelectFile(path);
             } else {
-                dialogService.showErrorDialogAndWait(Localization.lang("File not found"));
+                dialogService.showErrorDialogAndWait(Localization.lang("Cannot open folder as the file is an online link."));
             }
         } catch (IOException ex) {
             LOGGER.debug("Cannot open folder", ex);
@@ -251,7 +265,7 @@ public void moveToDefaultDirectory() {
 
         // Get target folder
         Optional<Path> fileDir = databaseContext.getFirstExistingFileDir(filePreferences);
-        if (!fileDir.isPresent()) {
+        if (fileDir.isEmpty()) {
             dialogService.showErrorDialogAndWait(Localization.lang("Move file"), Localization.lang("File directory is not set or does not exist!"));
             return;
         }
@@ -320,7 +334,7 @@ public void moveToDefaultDirectoryAndRename() {
     public boolean delete() {
         Optional<Path> file = linkedFile.findIn(databaseContext, filePreferences);
 
-        if (!file.isPresent()) {
+        if (file.isEmpty()) {
             LOGGER.warn("Could not find file " + linkedFile.getLink());
             return true;
         }
@@ -365,7 +379,7 @@ public void writeXMPMetadata() {
         // Localization.lang("Writing XMP metadata...")
         BackgroundTask<Void> writeTask = BackgroundTask.wrap(() -> {
             Optional<Path> file = linkedFile.findIn(databaseContext, filePreferences);
-            if (!file.isPresent()) {
+            if (file.isEmpty()) {
                 // TODO: Print error message
                 // Localization.lang("PDF does not exist");
             } else {
@@ -455,4 +469,6 @@ private Optional<ExternalFileType> inferFileTypeFromURL(String url) {
     public LinkedFile getFile() {
         return linkedFile;
     }
+
+    public ValidationStatus fileExistsValidationStatus() { return fileExistsValidator.getValidationStatus(); }
 }
diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
@@ -72,7 +72,8 @@ public LinkedFilesEditor(Field field, DialogService dialogService, BibDatabaseCo
                    .withOnMouseClickedEvent(this::handleItemMouseClick)
                    .setOnDragDetected(this::handleOnDragDetected)
                    .setOnDragDropped(this::handleOnDragDropped)
-                   .setOnDragOver(this::handleOnDragOver);
+                   .setOnDragOver(this::handleOnDragOver)
+                   .withValidation(LinkedFileViewModel::fileExistsValidationStatus);
 
         listView.setCellFactory(cellFactory);
 

diff --git a/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java b/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java
@@ -11,7 +11,6 @@
 import org.jabref.gui.fieldeditors.LinkedFileViewModel;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.entry.BibEntry;
-import org.jabref.model.entry.field.StandardField;
 import org.jabref.preferences.PreferencesService;
 
 public class OpenExternalFileAction extends SimpleCommand {
@@ -25,7 +24,7 @@ public OpenExternalFileAction(DialogService dialogService, StateManager stateMan
         this.stateManager = stateManager;
         this.preferencesService = preferencesService;
 
-        this.executable.bind(ActionHelper.isFieldSetForSelectedEntry(StandardField.FILE, stateManager)
+        this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferencesService)
                 .and(ActionHelper.needsEntriesSelected(1, stateManager)));
     }
 

diff --git a/src/main/java/org/jabref/gui/maintable/OpenFolderAction.java b/src/main/java/org/jabref/gui/maintable/OpenFolderAction.java
@@ -7,7 +7,6 @@
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.fieldeditors.LinkedFileViewModel;
-import org.jabref.model.entry.field.StandardField;
 import org.jabref.preferences.PreferencesService;
 
 public class OpenFolderAction extends SimpleCommand {
@@ -21,7 +20,7 @@ public OpenFolderAction(DialogService dialogService, StateManager stateManager,
         this.stateManager = stateManager;
         this.preferencesService = preferencesService;
 
-        this.executable.bind(ActionHelper.isFieldSetForSelectedEntry(StandardField.FILE, stateManager));
+        this.executable.bind(ActionHelper.isFilePresentForSelectedEntry(stateManager, preferencesService));
     }
 
     @Override

diff --git a/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java b/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java
@@ -22,6 +22,7 @@
 import org.jabref.gui.icon.JabRefIcon;
 import org.jabref.model.strings.StringUtil;
 
+import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
 import org.fxmisc.easybind.Subscription;
 
 /**
@@ -31,6 +32,8 @@
  */
 public class ViewModelListCellFactory<T> implements Callback<ListView<T>, ListCell<T>> {
 
+    private static final PseudoClass INVALID_PSEUDO_CLASS = PseudoClass.getPseudoClass("invalid");
+
     private Callback<T, String> toText;
     private Callback<T, Node> toGraphic;
     private Callback<T, Tooltip> toTooltip;
@@ -43,6 +46,7 @@ public class ViewModelListCellFactory<T> implements Callback<ListView<T>, ListCe
     private BiConsumer<T, ? super DragEvent> toOnDragExited;
     private BiConsumer<T, ? super DragEvent> toOnDragOver;
     private Map<PseudoClass, Callback<T, ObservableValue<Boolean>>> pseudoClasses = new HashMap<>();
+    private Callback<T, ValidationStatus> validationStatusProperty;
 
     public ViewModelListCellFactory<T> withText(Callback<T, String> toText) {
         this.toText = toText;
@@ -66,10 +70,7 @@ public ViewModelListCellFactory<T> withIcon(Callback<T, JabRefIcon> toIcon) {
     }
 
     public ViewModelListCellFactory<T> withIcon(Callback<T, JabRefIcon> toIcon, Callback<T, Color> toColor) {
-        this.toGraphic = viewModel -> {
-
-            return toIcon.call(viewModel).withColor(toColor.call(viewModel)).getGraphicNode();
-        };
+        this.toGraphic = viewModel -> toIcon.call(viewModel).withColor(toColor.call(viewModel)).getGraphicNode();
         return this;
     }
 
@@ -134,6 +135,11 @@ public ViewModelListCellFactory<T> withPseudoClass(PseudoClass pseudoClass, Call
         return this;
     }
 
+    public ViewModelListCellFactory<T> withValidation(Callback<T, ValidationStatus> validationStatusProperty) {
+        this.validationStatusProperty = validationStatusProperty;
+        return this;
+    }
+
     public void install(ComboBox<T> comboBox) {
         comboBox.setButtonCell(this.call(null));
         comboBox.setCellFactory(this);
@@ -146,7 +152,7 @@ public void install(ListView<T> listView) {
     @Override
     public ListCell<T> call(ListView<T> param) {
 
-        return new ListCell<T>() {
+        return new ListCell<>() {
 
             List<Subscription> subscriptions = new ArrayList<>();
 
@@ -203,6 +209,12 @@ protected void updateItem(T item, boolean empty) {
                         Subscription subscription = BindingsHelper.includePseudoClassWhen(this, pseudoClassWithCondition.getKey(), condition);
                         subscriptions.add(subscription);
                     }
+                    if (validationStatusProperty != null) {
+                        validationStatusProperty.call(viewModel).getHighestMessage().ifPresent(message -> {
+                            setTooltip(new Tooltip(message.getMessage()));
+                            subscriptions.add(BindingsHelper.includePseudoClassWhen(this, INVALID_PSEUDO_CLASS, validationStatusProperty.call(viewModel).validProperty().not()));
+                        });
+                    }
                 }
             }
         };

diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java
@@ -918,7 +918,7 @@ public Optional<FieldChange> setFiles(List<LinkedFile> files) {
     public List<LinkedFile> getFiles() {
         // Extract the path
         Optional<String> oldValue = getField(StandardField.FILE);
-        if (!oldValue.isPresent()) {
+        if (oldValue.isEmpty()) {
             return new ArrayList<>(); // Return new ArrayList because emptyList is immutable
         }
 

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -102,6 +102,8 @@ Cannot\ create\ group=Cannot create group
 
 Cannot\ create\ group.\ Please\ create\ a\ library\ first.=Cannot create group. Please create a library first.
 
+Cannot\ open\ folder\ as\ the\ file\ is\ an\ online\ link.=Cannot open folder as the file is an online link.
+
 case\ insensitive=case insensitive
 
 case\ sensitive=case sensitive