You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "vlsi (via GitHub)" <gi...@apache.org> on 2023/05/08 07:50:06 UTC
[GitHub] [jmeter] vlsi opened a new issue, #5895: Split JMeterGUIComponent into metadata and UI parts
vlsi opened a new issue, #5895:
URL: https://github.com/apache/jmeter/issues/5895
### 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:
```java
/**
* 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_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on issue #5895: Split JMeterGUIComponent into metadata and UI parts
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on issue #5895:
URL: https://github.com/apache/jmeter/issues/5895#issuecomment-1538422010
Sample diff for "a" for `ArgumentPanel`:
```diff
@@ -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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on issue #5895: Split JMeterGUIComponent into metadata and UI parts
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on issue #5895:
URL: https://github.com/apache/jmeter/issues/5895#issuecomment-1542499601
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on issue #5895: Split JMeterGUIComponent into metadata and UI parts
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on issue #5895:
URL: https://github.com/apache/jmeter/issues/5895#issuecomment-1540436616
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on issue #5895: Split JMeterGUIComponent into metadata and UI parts
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on issue #5895:
URL: https://github.com/apache/jmeter/issues/5895#issuecomment-1538429497
Here's the alternative for `TestBean` (== client code `implements TestBean` so JMeter spawns UI via `TestBeanGUI`):
```diff
@@ -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
```java
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;
}
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org