Split JMeterGUIComponent into metadata and UI parts

Use case

Currently, JMeterGUIComponent mixes several responsibilities: 1) Create and serve UI 2) Provide metadata on the component: #getStaticLabel, #getLabelResource, #getDocAnchor, #getMenuCategories

JMeter, needs to build menus on the startup, so it needs to discover elements, their labels, and the categories. Unfortunately, most implementations create UI elements in their constructors, so it would take a lot of time if JMeter instantiated all JMeterGUIComponents on the startup.

Instantiating the UI in the constructor blocks migration to ServiceLoader approach for component lookup as ServiceLoader always instantiates the services.

See

https://github.com/apache/jmeter/pull/5885

Possible solution

a) Split "metadata" part into a different interface.

For instance:

/**
 * Metadata for JMeter GUI components.
 */
interface GuiComponentRegistration {
    Class<?> implementationClass();
    String labelResource();
    default String resourceBundle() {
        return "";
    }
    default List<String> actionGroups() {
        return Collections.emptyList();
    }
}

// Sample component implementation

@AutoService(GuiComponentRegistration.class)
class WhileControllerComponentRegistration implements GuiComponentRegistration {
    @Override
    public Class<?> implementationClass() {
        return WhileControllerGui.class;
    }

    @Override
    public String labelResource() {
        return "while_controller_title";
    }
}

The implementation WhileControllerGui could delegate WhileControllerGui#labelResource to WhileControllerComponentRegistration service.

For instance, we could add AbstractJMeterGuiComponent#setMetadata(GuiComponentRegistration) method, and we could provide default implementations of AbstractJMeterGuiComponent#labelResource.

Yet another possibility is to add AbstractJMeterGuiComponent(GuiComponentRegistration) constructor and deprecate the old one. That would enable detecting non-migrated code at the compile time.

b) Move UI instantiation out of constructors

An alternative option is to move init() methods out of constructors and add the JMeterGUIComponent#createUI method to create all the UI components when needed.

I am not sure that would work well:

class AbstractJMeterGuiComponent extends JPanel, so it would trigger JPanel instantiation in any case, so it would impact the startup time
It would be impossible to use private final JTextArea commentField =... fields. Code complexity would increase as it would have to use nullable fields

Possible workarounds

No response

JMeter Version

5.5

Java Version

No response

OS Version

No response

Sample diff for "a" for ArgumentPanel:

@@ -60,8 +64,19 @@ import org.apache.jorphan.reflect.Functor;
  * for some other component.
  *
  */
-@TestElementMetadata(labelResource = "user_defined_variables")
 public class ArgumentsPanel extends AbstractConfigGui implements ActionListener {
+    @AutoService(JMeterGUIComponentMetadata.class)
+    public static class Metadata implements JMeterGUIComponentMetadata {
+        @Override
+        public Class<? extends JMeterGUIComponent> getImplementationClass() {
+            return ArgumentsPanel.class;
+        }
+
+        @Override
+        public String getLabelResource() {
+            return "user_defined_variables";
+        }
+    }

     private static final long serialVersionUID = 241L;

@@ -251,6 +266,7 @@ public class ArgumentsPanel extends AbstractConfigGui implements ActionListener
      */
     public ArgumentsPanel(String label, Color bkg, boolean enableUpDown, boolean standalone, ObjectTableModel model,
             boolean disableButtons, Function<String[], Argument> argCreator) {
+        setGUIComponentMetadata(new Metadata());
         tableLabel = new JLabel(label);
         this.enableUpDown = enableUpDown;
         this.disableButtons = disableButtons;
@@ -276,11 +292,6 @@ public class ArgumentsPanel extends AbstractConfigGui implements ActionListener
         return null;
     }

-    @Override
-    public String getLabelResource() {
-        return "user_defined_variables"; // $NON-NLS-1$
-    }
-

setGUIComponentMetadata(new Metadata()); is needed in the constructor for backward compatibility: users might have created new ArgumentsPanel(), and they expect the component to be workable in that case.

Here's the alternative for TestBean (== client code implements TestBean so JMeter spawns UI via TestBeanGUI):

@@ -52,6 +56,13 @@ public class BSFSampler extends BSFTestElement implements Sampler, TestBean, Con

     private static final Logger log = LoggerFactory.getLogger(BSFSampler.class);

+    @AutoService(JMeterGUIComponentMetadata.class)
+    public static class Metadata extends TestBeanGUIComponentMetadata {
+        public Metadata() {
+            super(BSFSampler.class);
+        }
+    }
+
     public BSFSampler() {
         super();
     }

TestBeanGUIComponentMetadata will be JMeter-provided class like

public class TestBeanGUIComponentMetadata implements JMeterGUIComponentMetadata {
    private final Class<? extends TestBean> clazz;

    public TestBeanGUIComponentMetadata(Class<? extends TestBean> clazz) {
        this.clazz = clazz;
    }

    @Override
    public final Class<? extends JMeterGUIComponent> getImplementationClass() {
        return TestBeanGUI.class;
    }
...

I just realized that with the new interface for JMeterGUIComponentMetadata we could revisit the API.

For instance, currently, TestBean instances have to use displayName for the label of the element. At the same time, the other elements (e.g. CounterConfigGui) use org.apache.jmeter.resources.messages resource bundle from the :src:core module.

I suggest that we move resources from org.apache.jmeter.resources.messages into individual bunles. For instance resources for CounterConfigGui could be moved from :src:core / org.apache.jmeter.resources.messages to /src/components/src/resources/.../CounterResources.properties bundle.

In other words, we could enforce that if a component uses resource bundles for managing resources, then the component should use displayName for the label. That would unify the way TestBean UI and JMeterGUIComponent UI works.

Here's an exception:

Caused by: java.lang.UnsupportedOperationException: Implement getGUIComponentMetadata() in class org.apache.jmeter.config.gui.ArgumentsPanel
    at org.apache.jmeter.gui.JMeterGUIComponent.getLabelResource(JMeterGUIComponent.java:122)
    at org.apache.jmeter.gui.AbstractJMeterGuiComponent.getStaticLabel(AbstractJMeterGuiComponent.java:370)
    at org.apache.jmeter.gui.AbstractJMeterGuiComponent.initGui(AbstractJMeterGuiComponent.java:243)
    at org.apache.jmeter.gui.AbstractJMeterGuiComponent.init(AbstractJMeterGuiComponent.java:248)
    at org.apache.jmeter.gui.AbstractJMeterGuiComponent.<init>(AbstractJMeterGuiComponent.java:95)
    at org.apache.jmeter.config.gui.AbstractConfigGui.<init>(AbstractConfigGui.java:33)
    at org.apache.jmeter.config.gui.ArgumentsPanel.<init>(ArgumentsPanel.java:262)
    at org.apache.jmeter.config.gui.ArgumentsPanel.<init>(ArgumentsPanel.java:210)
    at org.apache.jmeter.config.gui.ArgumentsPanel.<init>(ArgumentsPanel.java:156)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)

In other words, a "new-style" component ArgumentsPanel fails to instantiate since its superclass accesses getStaticLabel right in the superclass constructor.

Of course, we can't remove getStaticLabel method, and the superclasses might indeed call getStaticLabel.

So the way out seems to be adding another constructor that will receive JMeterGUIComponentMetadata parameter.

apache / jmeter