Streamline binding of UI elements to TestElement properties

src/core/src/main/java/org/apache/jmeter/control/gui/TestPlanGui.java

@@ -18,6 +18,7 @@
 package org.apache.jmeter.control.gui;
 
 import java.awt.BorderLayout;
+import java.util.Arrays;
 import java.util.Collection;
 
 import javax.swing.JComponent;
@@ -50,7 +51,7 @@ public class TestPlanGui extends AbstractJMeterGuiComponent {
     private static final long serialVersionUID = 240L;
 
     /**
-     * A checkbox allowing the user to specify whether or not JMeter should do
+     * A checkbox allowing the user to specify whether JMeter should do
      * functional testing.
      */
     private final JBooleanPropertyEditor functionalMode =
@@ -80,6 +81,13 @@ public TestPlanGui() {
         browseJar = new FileListPanel(JMeterUtils.getResString("test_plan_classpath_browse"), ".jar"); // $NON-NLS-1$ $NON-NLS-2$
         argsPanel = new ArgumentsPanel(JMeterUtils.getResString("user_defined_variables")); // $NON-NLS-1$
         init();
+        bindingGroup.addAll(
+                Arrays.asList(
+                        functionalMode,
+                        serializedMode,
+                        tearDownOnShutdown
+                )
+        );
     }
 
     /**
@@ -117,26 +125,33 @@ public JPopupMenu createPopupMenu() {
         return pop;
     }
 
-    /* Implements JMeterGUIComponent.createTestElement() */
     @Override
-    public TestElement createTestElement() {
-        TestPlan tp = new TestPlan();
-        modifyTestElement(tp);
-        return tp;
+    public TestElement makeTestElement() {
+        return new TestPlan();
+    }
+
+    @Override
+    public void assignDefaultValues(TestElement element) {
+        super.assignDefaultValues(element);
+        TestPlan tp = (TestPlan) element;
+        tp.setUserDefinedVariables((Arguments) argsPanel.createTestElement());
     }
 
     /* Implements JMeterGUIComponent.modifyTestElement(TestElement) */
     @Override
     public void modifyTestElement(TestElement plan) {
-        super.configureTestElement(plan);
+        super.modifyTestElement(plan);
         if (plan instanceof TestPlan) {
             TestPlan tp = (TestPlan) plan;
-            functionalMode.updateElement(tp);
-            tearDownOnShutdown.updateElement(tp);
-            serializedMode.updateElement(tp);
             // TODO: set expression to TestPlan somehow
             tp.setUserDefinedVariables((Arguments) argsPanel.createTestElement());
-            tp.setTestPlanClasspathArray(browseJar.getFiles());
+            String[] files = browseJar.getFiles();
+            if (files.length == 0) {
+                // Remove property if it is empty
+                tp.setTestPlanClasspath(null);
+            } else {
+                tp.setTestPlanClasspathArray(files);
+            }
         }
     }
 
@@ -172,9 +187,6 @@ public void configure(TestElement el) {
         super.configure(el);
         if (el instanceof TestPlan) {
             TestPlan tp = (TestPlan) el;
-            functionalMode.updateUi(tp);
-            serializedMode.updateUi(tp);
-            tearDownOnShutdown.updateUi(tp);
             final JMeterProperty udv = tp.getUserDefinedVariablesAsProperty();
             if (udv != null) {
                 argsPanel.configure((Arguments) udv.getObjectValue());
@@ -208,9 +220,6 @@ private void init() { // WARNING: called from ctor so must not be overridden (i.
     @Override
     public void clearGui() {
         super.clearGui();
-        functionalMode.reset();
-        serializedMode.reset();
-        tearDownOnShutdown.reset();
         argsPanel.clear();
         browseJar.clearFiles();
     }

src/core/src/main/java/org/apache/jmeter/control/gui/TransactionControllerGui.java

@@ -17,12 +17,11 @@
 
 package org.apache.jmeter.control.gui;
 
-import javax.swing.JCheckBox;
-
 import org.apache.jmeter.control.TransactionController;
+import org.apache.jmeter.control.TransactionControllerSchema;
 import org.apache.jmeter.gui.GUIMenuSortOrder;
+import org.apache.jmeter.gui.JBooleanPropertyEditor;
 import org.apache.jmeter.gui.TestElementMetadata;
-import org.apache.jmeter.gui.util.CheckBoxPanel;
 import org.apache.jmeter.testelement.TestElement;
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jorphan.gui.layout.VerticalLayout;
@@ -37,40 +36,36 @@ public class TransactionControllerGui extends AbstractControllerGui {
     private static final long serialVersionUID = 240L;
 
     /** If selected, then generate parent sample, otherwise as per original controller */
-    private JCheckBox generateParentSample;
+    private final JBooleanPropertyEditor generateParentSample =
+            new JBooleanPropertyEditor(
+                    TransactionControllerSchema.INSTANCE.getGenearteParentSample(),
+                    JMeterUtils.getResString("transaction_controller_parent"));
 
     /** if selected, add duration of timers to total runtime */
-    private JCheckBox includeTimers;
+    private final JBooleanPropertyEditor includeTimers =
+            new JBooleanPropertyEditor(
+                    TransactionControllerSchema.INSTANCE.getIncludeTimers(),
+                    JMeterUtils.getResString("transaction_controller_include_timers"));
 
     /**
      * Create a new TransactionControllerGui instance.
      */
     public TransactionControllerGui() {
         init();
+        bindingGroup.add(generateParentSample);
+        bindingGroup.add(includeTimers);
     }
 
     @Override
-    public TestElement createTestElement() {
-        TransactionController lc = new TransactionController();
-        lc.setIncludeTimers(false); // change default for new test elements
-        configureTestElement(lc);
-        return lc;
-    }
-
-    @Override
-    public void configure(TestElement el) {
-        super.configure(el);
-        generateParentSample.setSelected(((TransactionController) el).isGenerateParentSample());
-        includeTimers.setSelected(((TransactionController) el).isIncludeTimers());
+    public TestElement makeTestElement() {
+        return new TransactionController();
     }
 
     @Override
-    public void modifyTestElement(TestElement el) {
-        configureTestElement(el);
-        ((TransactionController) el).setGenerateParentSample(generateParentSample.isSelected());
-        TransactionController tc = (TransactionController) el;
-        tc.setGenerateParentSample(generateParentSample.isSelected());
-        tc.setIncludeTimers(includeTimers.isSelected());
+    public void assignDefaultValues(TestElement element) {
+        super.assignDefaultValues(element);
+        // See https://github.com/apache/jmeter/issues/3282
+        ((TransactionController) element).setIncludeTimers(false);
     }
 
     @Override
@@ -85,9 +80,7 @@ private void init() { // WARNING: called from ctor so must not be overridden (i.
         setLayout(new VerticalLayout(5, VerticalLayout.BOTH, VerticalLayout.TOP));
         setBorder(makeBorder());
         add(makeTitlePanel());
-        generateParentSample = new JCheckBox(JMeterUtils.getResString("transaction_controller_parent")); // $NON-NLS-1$
-        add(CheckBoxPanel.wrap(generateParentSample));
-        includeTimers = new JCheckBox(JMeterUtils.getResString("transaction_controller_include_timers"), true); // $NON-NLS-1$
-        add(CheckBoxPanel.wrap(includeTimers));
+        add(generateParentSample);
+        add(includeTimers);
     }
 }

src/core/src/main/java/org/apache/jmeter/gui/AbstractJMeterGuiComponent.java

@@ -35,9 +35,10 @@
 import javax.swing.JTextField;
 import javax.swing.border.Border;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.jmeter.gui.util.VerticalPanel;
 import org.apache.jmeter.testelement.TestElement;
-import org.apache.jmeter.testelement.property.StringProperty;
+import org.apache.jmeter.testelement.TestElementSchema;
 import org.apache.jmeter.util.JMeterUtils;
 import org.apache.jmeter.visualizers.Printable;
 import org.apache.jorphan.gui.JFactory;
@@ -83,6 +84,13 @@ public abstract class AbstractJMeterGuiComponent extends JPanel implements JMete
 
     private final JTextArea commentField = JFactory.tabMovesFocus(new JTextArea());
 
+    /**
+     * Stores a collection of property editors, so GuiCompoenent can have default implementations that
+     * update the UI fields based on {@link TestElement} properties and vice versa.
+     */
+    @API(status = EXPERIMENTAL, since = "5.6.3")
+    protected final BindingGroup bindingGroup = new BindingGroup();
+
     /**
      * When constructing a new component, this takes care of basic tasks like
      * setting up the Name Panel and assigning the class's static label as the
@@ -205,6 +213,7 @@ public void configure(TestElement element) {
         setName(element.getName());
         enabled = element.isEnabled();
         commentField.setText(element.getComment());
+        bindingGroup.updateUi(element);
     }
 
     /**
@@ -228,28 +237,52 @@ private void init() {
         initGui();
     }
 
+    @Override
+    @API(status = EXPERIMENTAL, since = "5.6.3")
+    public void modifyTestElement(TestElement element) {
+        JMeterGUIComponent.super.modifyTestElement(element);
+        modifyTestElementEnabledAndComment(element);
+        bindingGroup.updateElement(element);
+    }
+
     /**
      * This provides a convenience for extenders when they implement the
      * {@link JMeterGUIComponent#modifyTestElement(TestElement)} method. This
      * method will set the name, gui class, and test class for the created Test
      * Element. It should be called by every extending class when
      * creating/modifying Test Elements, as that will best assure consistent
      * behavior.
+     * <p>Deprecation notice: most likely you do not need the method, and you should
+     * override {@link #modifyTestElement(TestElement)} instead</p>
      *
      * @param mc
      *            the TestElement being created.
      */
+    @API(status = DEPRECATED, since = "5.6.3")
     protected void configureTestElement(TestElement mc) {
-        mc.setName(getName());
-
-        mc.setProperty(new StringProperty(TestElement.GUI_CLASS, this.getClass().getName()));
-
-        mc.setProperty(new StringProperty(TestElement.TEST_CLASS, mc.getClass().getName()));
+        mc.setName(StringUtils.defaultIfEmpty(getName(), null));
+        TestElementSchema schema = TestElementSchema.INSTANCE;
+        mc.set(schema.getGuiClass(), getClass());
+        mc.set(schema.getTestClass(), mc.getClass());
+        modifyTestElementEnabledAndComment(mc);
+    }
 
+    /**
+     * Assigns basic fields from UI to the test element: name, comments, gui class, and the registered editors.
+     *
+     * @param mc test element
+     */
+    private void modifyTestElementEnabledAndComment(TestElement mc) {
         // This stores the state of the TestElement
         log.debug("setting element to enabled: {}", enabled);
-        mc.setEnabled(enabled);
-        mc.setComment(getComment());
+        // We can skip storing "enabled" state if it's true, as it's default value.
+        // JMeter removes disabled elements early from the tree, so configuration elements
+        // with enabled=false (~HTTP Request Defaults) can't unexpectedly override the regular ones
+        // like HTTP Request.
+        mc.set(TestElementSchema.INSTANCE.getEnabled(), enabled ? null : Boolean.FALSE);
+        // Note: we can't use editors for "comments" as getComments() is not a final method, so plugins might
+        // override it and provide a different implementation.
+        mc.setComment(StringUtils.defaultIfEmpty(getComment(), null));
     }
 
     /**