jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1332389 - in /jmeter/trunk/src: core/org/apache/jmeter/config/gui/ArgumentsPanel.java protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.ja
Date Mon, 30 Apr 2012 23:07:56 GMT
On 30 April 2012 23:12, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> Hello Sebb,
> Not sure I understand clearly what you want me to change.
> If you drop initializeTableModel() I am not sure HTTPArgumentsPanel will
> work but I may be wrong.

In that case, it would need to call the super-class passing in the HTTP model.

> Feel free to do the changes on your side, it will be clearer, I will then
> review it.

I've reverted the changes so HTTPArgumentsPanel is the same as
previously, as is the API.

==

I'm not sure we need a separate protected method to create the table model.
The class is not much use without the model, and it has to be created
before much else can be done.
So it might as well be done in the constructor(s).
We could still keep the initializeTableModel() method, but make it
private and have it return the model.

The ctor can then set up the model by calling the method, and the
tableModel field can be made final - I think it can also be made
private.

> Thanks
> Regards
> Philippe
>
> On Tue, May 1, 2012 at 12:02 AM, sebb <sebbaz@gmail.com> wrote:
>
>> On 30 April 2012 22:03,  <pmouawad@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Mon Apr 30 21:03:27 2012
>> > New Revision: 1332389
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1332389&view=rev
>> > Log:
>> > Bug 53164 - New System Sampler
>> > Removed useless name column
>>
>> I'm not happy with the changes to the (HTTP)ArgumentsPanel classes.
>>
>> I think it would have been possible to implement the fix without
>> needing to change HTTPArguments.
>> Also, the model is ignored if standalone is true; that seems
>> unnecessarily restrictive.
>>
>> I think the code could be something like:
>>
>> ctor()
>> tableModel = model;
>>
>> initializeTableModel()
>> if (tableModel == null) {
>>   if (standalone) {
>>   } else {
>>   }
>> }
>>
>> Alternatively, if we are going to change the protected
>> initializeTableModel() method, I think it could just be dropped
>> entirely, and the model could be created by the ctor.
>>
>> > Modified:
>> >    jmeter/trunk/src/core/org/apache/jmeter/config/gui/ArgumentsPanel.java
>> >
>>  jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java
>> >
>>  jmeter/trunk/src/protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.java
>> >
>> > Modified:
>> jmeter/trunk/src/core/org/apache/jmeter/config/gui/ArgumentsPanel.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/config/gui/ArgumentsPanel.java?rev=1332389&r1=1332388&r2=1332389&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/core/org/apache/jmeter/config/gui/ArgumentsPanel.java
>> (original)
>> > +++
>> jmeter/trunk/src/core/org/apache/jmeter/config/gui/ArgumentsPanel.java Mon
>> Apr 30 21:03:27 2012
>> > @@ -89,7 +89,7 @@ public class ArgumentsPanel extends Abst
>> >     private Color background;
>> >
>> >     /**
>> > -     * Boolean indicating whether this component is a standalong
>> component or it
>> > +     * Boolean indicating whether this component is a standalone
>> component or it
>> >      * is intended to be used as a subpanel for another component.
>> >      */
>> >     private final boolean standalone;
>> > @@ -175,11 +175,23 @@ public class ArgumentsPanel extends Abst
>> >      * @param standalone is standalone
>> >      */
>> >     public ArgumentsPanel(String label, Color bkg, boolean enableUpDown,
>> boolean standalone) {
>> > +        this(label, bkg, enableUpDown, standalone, null);
>> > +    }
>> > +
>> > +    /**
>> > +     * Create a new ArgumentsPanel with a border and color background
>> > +     * @param label text for label
>> > +     * @param bkg background colour
>> > +     * @param enableUpDown Add up/down buttons
>> > +     * @param standalone is standalone
>> > +     * @param columns
>> > +     */
>> > +    public ArgumentsPanel(String label, Color bkg, boolean
>> enableUpDown, boolean standalone, ObjectTableModel model) {
>> >         tableLabel = new JLabel(label);
>> >         this.enableUpDown = enableUpDown;
>> >         this.background = bkg;
>> >         this.standalone = standalone;
>> > -        init();
>> > +        init(model);
>> >     }
>> >
>> >     /**
>> > @@ -532,8 +544,9 @@ public class ArgumentsPanel extends Abst
>> >
>> >     /**
>> >      * Initialize the table model used for the arguments table.
>> > +     * @param model ObjectTableModel
>> >      */
>> > -    protected void initializeTableModel() {
>> > +    protected void initializeTableModel(ObjectTableModel model) {
>> >         if(standalone) {
>> >             tableModel = new ObjectTableModel(new String[] {
>> COLUMN_RESOURCE_NAMES_0, COLUMN_RESOURCE_NAMES_1, COLUMN_RESOURCE_NAMES_2 },
>> >                     Argument.class,
>> > @@ -547,21 +560,25 @@ public class ArgumentsPanel extends Abst
>> >                     new Functor("setDescription") },  // $NON-NLS-1$
>> >                     new Class[] { String.class, String.class,
>> String.class });
>> >         } else {
>> > -        tableModel = new ObjectTableModel(new String[] {
>> COLUMN_RESOURCE_NAMES_0, COLUMN_RESOURCE_NAMES_1 },
>> > -                Argument.class,
>> > -                new Functor[] {
>> > -                new Functor("getName"), // $NON-NLS-1$
>> > -                new Functor("getValue") },  // $NON-NLS-1$
>> > -                new Functor[] {
>> > -                new Functor("setName"), // $NON-NLS-1$
>> > -                new Functor("setValue") }, // $NON-NLS-1$
>> > -                new Class[] { String.class, String.class });
>> > -    }
>> > +            if(model != null) {
>> > +                tableModel = model;
>> > +            } else  {
>> > +                tableModel = new ObjectTableModel(new String[] {
>> COLUMN_RESOURCE_NAMES_0, COLUMN_RESOURCE_NAMES_1 },
>> > +                    Argument.class,
>> > +                    new Functor[] {
>> > +                    new Functor("getName"), // $NON-NLS-1$
>> > +                    new Functor("getValue") },  // $NON-NLS-1$
>> > +                    new Functor[] {
>> > +                    new Functor("setName"), // $NON-NLS-1$
>> > +                    new Functor("setValue") }, // $NON-NLS-1$
>> > +                    new Class[] { String.class, String.class });
>> > +            }
>> > +        }
>> >     }
>> >
>> >     public static boolean testFunctors(){
>> >         ArgumentsPanel instance = new ArgumentsPanel();
>> > -        instance.initializeTableModel();
>> > +        instance.initializeTableModel(null);
>> >         return
>> instance.tableModel.checkFunctors(null,instance.getClass());
>> >     }
>> >
>> > @@ -576,11 +593,12 @@ public class ArgumentsPanel extends Abst
>> >
>> >     /**
>> >      * Create the main GUI panel which contains the argument table.
>> > +     * @param model ObjectTableModel
>> >      *
>> >      * @return the main GUI panel
>> >      */
>> > -    private Component makeMainPanel() {
>> > -        initializeTableModel();
>> > +    private Component makeMainPanel(ObjectTableModel model) {
>> > +        initializeTableModel(model);
>> >         table = new JTable(tableModel);
>> >         table.getTableHeader().setDefaultRenderer(new
>> HeaderAsPropertyRenderer());
>> >
>> table.setSelectionMode(ListSelectionModel.MULTIPLE_INTERVAL_SELECTION);
>> > @@ -658,8 +676,9 @@ public class ArgumentsPanel extends Abst
>> >
>> >     /**
>> >      * Initialize the components and layout of this component.
>> > +     * @param model ObjectTableModel
>> >      */
>> > -    private void init() {
>> > +    private void init(ObjectTableModel model) {
>> >         JPanel p = this;
>> >
>> >         if (standalone) {
>> > @@ -672,7 +691,7 @@ public class ArgumentsPanel extends Abst
>> >         p.setLayout(new BorderLayout());
>> >
>> >         p.add(makeLabelPanel(), BorderLayout.NORTH);
>> > -        p.add(makeMainPanel(), BorderLayout.CENTER);
>> > +        p.add(makeMainPanel(model), BorderLayout.CENTER);
>> >         // Force a minimum table height of 70 pixels
>> >         p.add(Box.createVerticalStrut(70), BorderLayout.WEST);
>> >         p.add(makeButtonPanel(), BorderLayout.SOUTH);
>> >
>> > Modified:
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java?rev=1332389&r1=1332388&r2=1332389&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/HTTPArgumentsPanel.java
>> Mon Apr 30 21:03:27 2012
>> > @@ -46,26 +46,30 @@ public class HTTPArgumentsPanel extends
>> >     private static final String INCLUDE_EQUALS = "include_equals";
>> //$NON-NLS-1$
>> >
>> >     @Override
>> > -    protected void initializeTableModel() {
>> > -        tableModel = new ObjectTableModel(new String[] {
>> > -                ArgumentsPanel.COLUMN_RESOURCE_NAMES_0,
>> ArgumentsPanel.COLUMN_RESOURCE_NAMES_1, ENCODE_OR_NOT, INCLUDE_EQUALS },
>> > -                HTTPArgument.class,
>> > -                new Functor[] {
>> > -                new Functor("getName"), //$NON-NLS-1$
>> > -                new Functor("getValue"), //$NON-NLS-1$
>> > -                new Functor("isAlwaysEncoded"), //$NON-NLS-1$
>> > -                new Functor("isUseEquals") }, //$NON-NLS-1$
>> > -                new Functor[] {
>> > -                new Functor("setName"), //$NON-NLS-1$
>> > -                new Functor("setValue"), //$NON-NLS-1$
>> > -                new Functor("setAlwaysEncoded"), //$NON-NLS-1$
>> > -                new Functor("setUseEquals") }, //$NON-NLS-1$
>> > -                new Class[] {String.class, String.class, Boolean.class,
>> Boolean.class });
>> > +    protected void initializeTableModel(ObjectTableModel model) {
>> > +        if(tableModel == null) {
>> > +            tableModel = new ObjectTableModel(new String[] {
>> > +                    ArgumentsPanel.COLUMN_RESOURCE_NAMES_0,
>> ArgumentsPanel.COLUMN_RESOURCE_NAMES_1, ENCODE_OR_NOT, INCLUDE_EQUALS },
>> > +                    HTTPArgument.class,
>> > +                    new Functor[] {
>> > +                    new Functor("getName"), //$NON-NLS-1$
>> > +                    new Functor("getValue"), //$NON-NLS-1$
>> > +                    new Functor("isAlwaysEncoded"), //$NON-NLS-1$
>> > +                    new Functor("isUseEquals") }, //$NON-NLS-1$
>> > +                    new Functor[] {
>> > +                    new Functor("setName"), //$NON-NLS-1$
>> > +                    new Functor("setValue"), //$NON-NLS-1$
>> > +                    new Functor("setAlwaysEncoded"), //$NON-NLS-1$
>> > +                    new Functor("setUseEquals") }, //$NON-NLS-1$
>> > +                    new Class[] {String.class, String.class,
>> Boolean.class, Boolean.class });
>> > +        } else {
>> > +            tableModel = model;
>> > +        }
>> >     }
>> >
>> >     public static boolean testFunctors(){
>> >         HTTPArgumentsPanel instance = new HTTPArgumentsPanel();
>> > -        instance.initializeTableModel();
>> > +        instance.initializeTableModel(null);
>> >         return
>> instance.tableModel.checkFunctors(null,instance.getClass());
>> >     }
>> >
>> >
>> > Modified:
>> jmeter/trunk/src/protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.java?rev=1332389&r1=1332388&r2=1332389&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/native/org/apache/jmeter/protocol/system/gui/SystemSamplerGui.java
>> Mon Apr 30 21:03:27 2012
>> > @@ -28,6 +28,7 @@ import javax.swing.BoxLayout;
>> >  import javax.swing.JCheckBox;
>> >  import javax.swing.JPanel;
>> >
>> > +import org.apache.jmeter.config.Argument;
>> >  import org.apache.jmeter.config.Arguments;
>> >  import org.apache.jmeter.config.gui.ArgumentsPanel;
>> >  import org.apache.jmeter.gui.util.VerticalPanel;
>> > @@ -36,6 +37,8 @@ import org.apache.jmeter.samplers.gui.Ab
>> >  import org.apache.jmeter.testelement.TestElement;
>> >  import org.apache.jmeter.util.JMeterUtils;
>> >  import org.apache.jorphan.gui.JLabeledTextField;
>> > +import org.apache.jorphan.gui.ObjectTableModel;
>> > +import org.apache.jorphan.reflect.Functor;
>> >
>> >  /**
>> >  * GUI for {@link SystemSampler}
>> > @@ -160,7 +163,14 @@ public class SystemSamplerGui extends Ab
>> >      * @return JPanel Arguments Panel
>> >      */
>> >     private JPanel makeArgumentsPanel() {
>> > -        argsPanel = new
>> ArgumentsPanel(JMeterUtils.getResString("arguments_panel_title"), true);
>> > +        argsPanel = new
>> ArgumentsPanel(JMeterUtils.getResString("arguments_panel_title"), null,
>> true, false ,
>> > +                new ObjectTableModel(new String[] {
>> ArgumentsPanel.COLUMN_RESOURCE_NAMES_1 },
>> > +                        Argument.class,
>> > +                        new Functor[] {
>> > +                        new Functor("getValue") },  // $NON-NLS-1$
>> > +                        new Functor[] {
>> > +                        new Functor("setValue") }, // $NON-NLS-1$
>> > +                        new Class[] {String.class }));
>> >         return argsPanel;
>> >     }
>> >
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message