Merge pull request #3173 from ControlSystemStudio/PR-3071-fix

Fix display context menu entries for open display action

Author: georgweiss

app/display/actions/src/main/java/org/csstudio/display/actions/OpenDisplayAction.java

@@ -15,6 +15,7 @@
 import org.csstudio.display.builder.model.persist.ModelWriter;
 import org.csstudio.display.builder.model.persist.XMLTags;
 import org.csstudio.display.builder.model.properties.ActionInfoBase;
+import org.csstudio.display.builder.model.spi.ActionHandler;
 import org.csstudio.display.builder.model.spi.ActionInfo;
 import org.csstudio.display.builder.representation.javafx.actionsdialog.ActionsDialog;
 import org.phoebus.framework.macros.Macros;
@@ -30,6 +31,8 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.ResourceBundle;
+import java.util.ServiceLoader;
+import java.util.concurrent.ExecutorService;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -215,17 +218,22 @@ public void setModifiers(final MouseEvent event) {
     }
 
     @Override
-    public List<MenuItem> getContextMenuItems(Widget widget) {
+    public List<MenuItem> getContextMenuItems(ExecutorService executorService, Widget widget) {
+        ActionHandler handler = getActionHandler();
         List<MenuItem> items = new ArrayList<>();
 
-        items.add(createMenuItem(widget, description));
+        // Default item
+        items.add(createMenuItem(executorService, widget, description));
 
         // Add variant for all the available Target types: Replace, new Tab, ...
         for (OpenDisplayAction.Target target : OpenDisplayAction.Target.values()) {
             if (target == OpenDisplayAction.Target.STANDALONE || target == this.target)
                 continue;
             // Mention non-default targets in the description
-            items.add(createMenuItem(widget, description + " (" + target + ")"));
+            MenuItem additionalItem = createMenuItem(widget, description + " (" + target + ")");
+            OpenDisplayAction openDisplayAction = new OpenDisplayAction(description, file, macros, target);
+            additionalItem.setOnAction(ae -> executorService.execute(() -> handler.handleAction(widget, openDisplayAction)));
+            items.add(additionalItem);
         }
 
         return items;

app/display/model/src/main/java/org/csstudio/display/builder/model/properties/ActionInfoBase.java

@@ -11,12 +11,14 @@
 import org.csstudio.display.builder.model.WidgetProperty;
 import org.csstudio.display.builder.model.macros.MacroHandler;
 import org.csstudio.display.builder.model.persist.XMLTags;
+import org.csstudio.display.builder.model.spi.ActionHandler;
 import org.csstudio.display.builder.model.spi.ActionInfo;
 
 import javax.xml.stream.XMLStreamWriter;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import java.util.concurrent.ExecutorService;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -46,6 +48,31 @@ public String getType() {
         return type;
     }
 
+    /**
+     * Creates a {@link MenuItem} for this action and configures the <code>onAction</code> handler
+     * to use associated {@link ActionHandler}.
+     *
+     * @param executorService A non-null {@link ExecutorService} used to execute the action using the associated {@link ActionHandler}.
+     * @param widget          Widget associated with the action
+     * @param description     Description set as text for the {@link MenuItem}
+     * @return A {@link MenuItem} ready to insert in a {@link javafx.scene.control.ContextMenu}.
+     * @throws RuntimeException if an {@link ActionHandler} cannot be located for the action.
+     */
+    protected MenuItem createMenuItem(final ExecutorService executorService, final Widget widget, final String description) {
+        ActionHandler actionHandler = getActionHandler();
+        MenuItem item = createMenuItem(widget, description);
+        item.setOnAction(ae -> executorService.execute(() -> actionHandler.handleAction(widget, this)));
+        return item;
+    }
+
+    /**
+     * Creates a {@link MenuItem} for this action, but does not configure the <code>onAction</code> handler.
+     *
+     * @param widget      Widget associated with the action
+     * @param description Description set as text for the {@link MenuItem}
+     * @return A {@link MenuItem} ready to insert in a {@link javafx.scene.control.ContextMenu}, but caller
+     * must define <code>onAction</code>.
+     */
     protected MenuItem createMenuItem(final Widget widget, final String description) {
         // Expand macros in action description
         String desc;
@@ -68,8 +95,6 @@ protected MenuItem createMenuItem(final Widget widget, final String description)
         return item;
     }
 
-
-
     @Override
     public String toString() {
         return description;
@@ -92,13 +117,14 @@ protected void writeDescriptionToXML(final XMLStreamWriter writer, String descri
     /**
      * Adds a single menu item corresponding to the action's description. If the {@link ActionInfo} implementation
      * needs additional items, it should override this method.
+     *
      * @param widget Widget associated with the context menu.
      * @return A list of {@link MenuItem}s.
      */
     @Override
-    public List<MenuItem> getContextMenuItems(Widget widget) {
+    public List<MenuItem> getContextMenuItems(ExecutorService executorService, Widget widget) {
         List<MenuItem> items = new ArrayList<>();
-        items.add(createMenuItem(widget, description));
+        items.add(createMenuItem(executorService, widget, description));
 
         return items;
     }

app/display/model/src/main/java/org/csstudio/display/builder/model/spi/ActionInfo.java

@@ -29,14 +29,17 @@
 import org.w3c.dom.Element;
 
 import javax.xml.stream.XMLStreamWriter;
-import java.util.Comparator;
 import java.util.List;
+import java.util.Optional;
+import java.util.ServiceLoader;
+import java.util.concurrent.ExecutorService;
 
 public interface ActionInfo extends Comparable<ActionInfo> {
 
     /**
      * If action type is not sufficient to determine match, implementations may add additional logic
      * to resolve match. For instance: legacy display formats may use a different string to define the action type.
+     *
      * @param actionId Action id, e.g. open_display.
      * @return <code>true</code> if the input string is implemented by the {@link ActionInfo}.
      */
@@ -64,8 +67,7 @@ default boolean matchesAction(String actionId) {
     String getDescription();
 
     /**
-     *
-     * @return
+     * @return Integer used to order actions in the display builder editor
      */
     default Integer getPriority() {
         return 100;
@@ -103,13 +105,18 @@ default void setModifiers(MouseEvent event) {
     }
 
     /**
-     * @return A {@link List} of {@link MenuItem}s for the action in a widget's context menu.
+     * @param executorService The {@link ExecutorService} responsible for running the action(s) defined
+     *                        by the returned {@link MenuItem}s.
+     * @param widget          The {@link Widget} defining the actions.
+     * @return A {@link List} of {@link MenuItem}s for the action in a widget's context menu. All {@link MenuItem}s
+     * must define the <code>onAction()</code>onAction() handler.
      * Defaults to <code>null</code>.
      */
-    default List<MenuItem> getContextMenuItems(Widget widget) {
+    default List<MenuItem> getContextMenuItems(ExecutorService executorService, Widget widget) {
         return null;
     }
 
+
     /**
      * @param widget The {@link Widget} associated with the action(s).
      * @return The editor UI particular for the {@link ActionInfo} implementation
@@ -130,15 +137,27 @@ default List<MenuItem> getContextMenuItems(Widget widget) {
 
     /**
      * Comparator for the sake of sorting {@link ActionInfo}s. Uses {@link ActionInfo#getDescription()}.
+     *
      * @param other the object to be compared.
      * @return Any occurrence of <code>null</code> in the {@link ActionInfo#getDescription()}
      * fields will return 0. Otherwise, comparison of {@link ActionInfo#getDescription()}.
      */
     @Override
-    default int compareTo(ActionInfo other){
-        if(getDescription() == null || other.getDescription() == null){
+    default int compareTo(ActionInfo other) {
+        if (getDescription() == null || other.getDescription() == null) {
             return 0;
         }
         return getDescription().compareTo(other.getDescription());
     }
+
+    /**
+     * @return An {@link ActionHandler} matching the action. If none can be found, a {@link RuntimeException} is thrown.
+     */
+    default ActionHandler getActionHandler() {
+        Optional<ServiceLoader.Provider<ActionHandler>> handler = ServiceLoader.load(ActionHandler.class).stream().filter(p -> p.get().matches(this)).findFirst();
+        if (handler.isEmpty()) {
+            throw new RuntimeException("No ActionHandler found for action " + getDescription());
+        }
+        return handler.get().get();
+    }
 }

app/display/runtime/src/main/java/org/csstudio/display/builder/runtime/app/ContextMenuSupport.java

@@ -21,7 +21,6 @@
 import org.csstudio.display.builder.model.spi.ActionInfo;
 import org.csstudio.display.builder.representation.ToolkitListener;
 import org.csstudio.display.builder.representation.javafx.widgets.JFXBaseRepresentation;
-import org.csstudio.display.builder.runtime.ActionUtil;
 import org.csstudio.display.builder.runtime.Messages;
 import org.csstudio.display.builder.runtime.RuntimeAction;
 import org.csstudio.display.builder.runtime.RuntimeUtil;
@@ -129,9 +128,8 @@ private void fillMenu(Runnable setFocus, final Widget widget) {
 
         // Widget actions
         for (ActionInfo info : widget.propActions().getValue().getActions()) {
-            List<MenuItem> actionMenuItems = info.getContextMenuItems(widget);
+            List<MenuItem> actionMenuItems = info.getContextMenuItems(RuntimeUtil.getExecutor(), widget);
             if (actionMenuItems != null) {
-                actionMenuItems.forEach(i -> i.setOnAction(e -> ActionUtil.handleAction(widget, info)));
                 items.addAll(actionMenuItems);
             }
         }