Fixed possible NPE in SpecialFieldAction

ShikunXiong · Feb 22, 2020 · a800bf4 · a800bf4
1 parent a5bf6b7
commit a800bf4
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 28 deletions.
diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -728,12 +728,14 @@ private MenuBar createMenu() {
         if (Globals.prefs.getBoolean(JabRefPreferences.SPECIALFIELDSENABLED)) {
             edit.getItems().addAll(
                     new SeparatorMenuItem(),
-                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.RANKING, factory, getCurrentBasePanel(), dialogService, stateManager),
-                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.RELEVANCE, factory, getCurrentBasePanel(), dialogService, stateManager),
-                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.QUALITY, factory, getCurrentBasePanel(), dialogService, stateManager),
-                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, getCurrentBasePanel(), dialogService, stateManager),
-                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, getCurrentBasePanel(), dialogService, stateManager),
-                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, getCurrentBasePanel(), dialogService, stateManager)
+                    // ToDo: SpecialField needs the active BasePanel to mark it as changed.
+                    //  Refactor BasePanel, should mark the BibDatabaseContext or the UndoManager as dirty instead!
+                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.RANKING, factory, this, dialogService, stateManager),
+                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.RELEVANCE, factory, this, dialogService, stateManager),
+                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.QUALITY, factory, this, dialogService, stateManager),
+                    SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, this, dialogService, stateManager),
+                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, this, dialogService, stateManager),
+                    SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, this, dialogService, stateManager)
             );
         }
 

diff --git a/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/src/main/java/org/jabref/gui/maintable/RightClickMenu.java
@@ -49,12 +49,14 @@ public static ContextMenu create(BibEntryTableViewModel entry, KeyBindingReposit
         contextMenu.getItems().add(new SeparatorMenuItem());
 
         if (Globals.prefs.getBoolean(JabRefPreferences.SPECIALFIELDSENABLED)) {
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.RANKING, factory, panel, dialogService, stateManager));
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.RELEVANCE, factory, panel, dialogService, stateManager));
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.QUALITY, factory, panel, dialogService, stateManager));
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, panel, dialogService, stateManager));
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, panel, dialogService, stateManager));
-            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, panel, dialogService, stateManager));
+            // ToDo: SpecialField needs the active BasePanel to mark it as changed.
+            //  Refactor BasePanel, should mark the BibDatabaseContext or the UndoManager as dirty instead!
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.RANKING, factory, panel.frame(), dialogService, stateManager));
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.RELEVANCE, factory, panel.frame(), dialogService, stateManager));
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.QUALITY, factory, panel.frame(), dialogService, stateManager));
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.getSpecialFieldSingleItem(SpecialField.PRINTED, factory, panel.frame(), dialogService, stateManager));
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.PRIORITY, factory, panel.frame(), dialogService, stateManager));
+            contextMenu.getItems().add(SpecialFieldMenuItemFactory.createSpecialFieldMenu(SpecialField.READ_STATUS, factory, panel.frame(), dialogService, stateManager));
         }
 
         contextMenu.getItems().add(new SeparatorMenuItem());
@@ -68,6 +70,7 @@ public static ContextMenu create(BibEntryTableViewModel entry, KeyBindingReposit
         contextMenu.getItems().add(new ChangeEntryTypeMenu().getChangeEntryTypeMenu(entry.getEntry(), panel.getBibDatabaseContext(), panel.getUndoManager()));
         contextMenu.getItems().add(factory.createMenuItem(StandardActions.MERGE_WITH_FETCHED_ENTRY, new MergeWithFetchedEntryAction(panel, dialogService, stateManager)));
         contextMenu.getItems().add(factory.createMenuItem(StandardActions.ATTACH_FILE, new AttachFileAction(panel, dialogService, stateManager, preferencesService)));
+        // ToDo: Refactor BasePanel, see ahead.
         contextMenu.getItems().add(factory.createMenuItem(StandardActions.MERGE_ENTRIES, new MergeEntriesAction(panel.frame(), dialogService, stateManager)));
 
         return contextMenu;

diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java
@@ -4,8 +4,8 @@
 import java.util.Objects;
 
 import org.jabref.Globals;
-import org.jabref.gui.BasePanel;
 import org.jabref.gui.DialogService;
+import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.ActionHelper;
 import org.jabref.gui.actions.SimpleCommand;
@@ -23,7 +23,7 @@
 public class SpecialFieldAction extends SimpleCommand {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(SpecialFieldAction.class);
-    private final BasePanel panel;
+    private final JabRefFrame frame;
     private final SpecialField specialField;
     private final String value;
     private final boolean nullFieldIfValueIsTheSame;
@@ -35,14 +35,14 @@ public class SpecialFieldAction extends SimpleCommand {
      * @param nullFieldIfValueIsTheSame - false also causes that doneTextPattern has two place holders %0 for the value and %1 for the sum of entries
      */
     public SpecialFieldAction(
-            BasePanel panel,
+            JabRefFrame frame,
             SpecialField specialField,
             String value,
             boolean nullFieldIfValueIsTheSame,
             String undoText,
             DialogService dialogService,
             StateManager stateManager) {
-        this.panel = panel;
+        this.frame = frame;
         this.specialField = specialField;
         this.value = value;
         this.nullFieldIfValueIsTheSame = nullFieldIfValueIsTheSame;
@@ -70,9 +70,9 @@ public void execute() {
             }
             ce.end();
             if (ce.hasEdits()) {
-                panel.getUndoManager().addEdit(ce);
-                panel.markBaseChanged();
-                panel.updateEntryEditorIfShowing();
+                frame.getCurrentBasePanel().getUndoManager().addEdit(ce);
+                frame.getCurrentBasePanel().markBaseChanged();
+                frame.getCurrentBasePanel().updateEntryEditorIfShowing();
                 String outText;
                 if (nullFieldIfValueIsTheSame || value == null) {
                     outText = getTextDone(specialField, Integer.toString(bes.size()));
@@ -92,7 +92,7 @@ public void execute() {
     private String getTextDone(SpecialField field, String... params) {
         Objects.requireNonNull(params);
 
-        SpecialFieldViewModel viewModel = new SpecialFieldViewModel(field, panel.getUndoManager());
+        SpecialFieldViewModel viewModel = new SpecialFieldViewModel(field, frame.getUndoManager());
 
         if (field.isSingleValueField() && (params.length == 1) && (params[0] != null)) {
             // Single value fields can be toggled only

diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldMenuItemFactory.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldMenuItemFactory.java
@@ -8,8 +8,8 @@
 import javafx.scene.control.MenuItem;
 
 import org.jabref.Globals;
-import org.jabref.gui.BasePanel;
 import org.jabref.gui.DialogService;
+import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.ActionFactory;
 import org.jabref.model.entry.field.SpecialField;
@@ -18,15 +18,15 @@
 import de.saxsys.mvvmfx.utils.commands.Command;
 
 public class SpecialFieldMenuItemFactory {
-    public static MenuItem getSpecialFieldSingleItem(SpecialField field, ActionFactory factory, BasePanel panel, DialogService dialogService, StateManager stateManager) {
+    public static MenuItem getSpecialFieldSingleItem(SpecialField field, ActionFactory factory, JabRefFrame frame, DialogService dialogService, StateManager stateManager) {
         SpecialFieldValueViewModel specialField = new SpecialFieldValueViewModel(field.getValues().get(0));
         return factory.createMenuItem(specialField.getAction(),
-                new SpecialFieldViewModel(field, Globals.undoManager).getSpecialFieldAction(field.getValues().get(0), panel, dialogService, stateManager));
+                new SpecialFieldViewModel(field, Globals.undoManager).getSpecialFieldAction(field.getValues().get(0), frame, dialogService, stateManager));
     }
 
-    public static Menu createSpecialFieldMenu(SpecialField field, ActionFactory factory, BasePanel panel, DialogService dialogService, StateManager stateManager) {
+    public static Menu createSpecialFieldMenu(SpecialField field, ActionFactory factory, JabRefFrame frame, DialogService dialogService, StateManager stateManager) {
         return createSpecialFieldMenu(field, factory, Globals.undoManager, specialField ->
-                new SpecialFieldViewModel(field, Globals.undoManager).getSpecialFieldAction(specialField.getValue(), panel, dialogService, stateManager));
+                new SpecialFieldViewModel(field, Globals.undoManager).getSpecialFieldAction(specialField.getValue(), frame, dialogService, stateManager));
     }
 
     public static Menu createSpecialFieldMenu(SpecialField field, ActionFactory factory, UndoManager undoManager, Function<SpecialFieldValueViewModel, Command> commandFactory) {

diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldViewModel.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldViewModel.java
@@ -7,8 +7,8 @@
 import javax.swing.undo.UndoManager;
 
 import org.jabref.Globals;
-import org.jabref.gui.BasePanel;
 import org.jabref.gui.DialogService;
+import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.Action;
 import org.jabref.gui.actions.StandardActions;
@@ -34,8 +34,8 @@ public SpecialField getField() {
         return field;
     }
 
-    public SpecialFieldAction getSpecialFieldAction(SpecialFieldValue value, BasePanel panel, DialogService dialogService, StateManager stateManager) {
-        return new SpecialFieldAction(panel, field, value.getFieldValue().orElse(null),
+    public SpecialFieldAction getSpecialFieldAction(SpecialFieldValue value, JabRefFrame frame, DialogService dialogService, StateManager stateManager) {
+        return new SpecialFieldAction(frame, field, value.getFieldValue().orElse(null),
                 // if field contains only one value, it has to be nulled
                 // otherwise, another setting does not empty the field
                 field.getValues().size() == 1,