Bug 51861

Created attachment 27553 [details]
Fix to the issue

Hello,
Here is the patch, your criticism are welcome.
Hope you can take it into account soon (in next release 2.5.2 if not this one).

Regards
Philippe Mouawad

Looks very useful.

Would it be possible to select the mode by clicking the tab instead of using the drop-down?

Just tried some moving around in the tree combined with switching the mode, and managed to lose the contents of the panes.

I think it needs a bit more work to ensure data is not lost.

Hello Sebb,
For your first question, I made it like this because I meant the 2 mode not to be a different presentation of parameters but a different kind of usage.

Regarding second question, can you give me the scenario that led to what you are saying. 
It may be a regular case, I mean I don't handle the switch of data presentation from one mode to the other because I don't think it is useful but I am open to your propositions.

Regards
Philippe

If you have a better ergonomy idea to avoid coming to this case I am open to it.
I though drop-down was a way to do this.

Regards
Philippe

Created attachment 27562 [details]
Test Plan

Example test plan that uses this feature.
The feature works like this for me:
- If I choose not /security.rpcs and switch from RAW Body to Parameters, then I will loose parameters (I can change it, but switching  From Parameter to RAW body will be harder to manage regarding and in GWT case I don't know why I would do that)
- If I choose /j_spring_security_check node, I am in Parameters mode and switching to other mode will give me an empty body

Regards
Philippe

(In reply to comment #5)
> If you have a better ergonomy idea to avoid coming to this case I am open to
> it.

When I first saw the GUI, I expected to be able to click on the tab name, i.e.
"Parameters" or "Post Body"

Have a look at View Results Tree - the Sampler Result panel allows selection of
"Raw" and "Parsed" just by clicking the tab.

> I though drop-down was a way to do this.

Yes, but it uses a lot of space, and is not visually related to the panels.

(In reply to comment #6)
> Created attachment 27562 [details]
> Test Plan

It does not seem to be a test plan

Whilst the HTTP Sampler remains selected, it shows any data in both panes.
One can switch easily between them.

However, as soon as one switches to another test element and back, the pane that was not selected is cleared.

That confused me, and I think it will confuse/annoy others.

(In reply to comment #7)
> (In reply to comment #5)
> > If you have a better ergonomy idea to avoid coming to this case I am open to
> > it.
> 
> When I first saw the GUI, I expected to be able to click on the tab name, i.e.
> "Parameters" or "Post Body"
> 
> Have a look at View Results Tree - the Sampler Result panel allows selection of
> "Raw" and "Parsed" just by clicking the tab.

Actually, it's trivial: just remove postBodyMode and ensure both postContentTabbedPane panels are left enabled.

> > I though drop-down was a way to do this.
> 
> Yes, but it uses a lot of space, and is not visually related to the panels.

Created attachment 27563 [details]
Updated patch without drop Down

Here's my version of the patch.

It still suffers from the problem of losing data when switching modes.

Great.
Thank you very much for all your notes and patch.
Do you want me to handle the switch or can we say it will work like this:
- If Raw body is selected and data filled and then user switches to Parameters he loses everything => So no more difference that will make him think it might be a bug
- Same in switch from Parameters to Raw Body

Or we implement the switch with the complexity of needing to parse new input data in RAW body.


Regards
Philippe

Created attachment 27564 [details]
The real test

I don't think we should ever lose data.

So maybe we don't allow the user to switch to RAW unless there are no existing parameters, or the parameters can be converted directly to RAW.

Likewise, switching away from RAW should be disallowed unless the data can be converted to Parameters (or there is no data).

There's another issue with the code at present.

HTTP generally uses CRLF line terminators; however Java uses LF (\n) only in the text area.

This means converting \n to \r\n on save, and reverting on restore, otherwwise the CRs start multiplying!

Not sure if this will have any side effects.

Should this conversion be optional?

Hello Sebb,
Regarding you N-1 note:
"Likewise, switching away from RAW should be disallowed unless the data can be
converted to Parameters (or there is no data)."

This is not so easy to implement because parsing this valid GWT query Body:
"7|0|8|http://www.xxxxxxxxxxxxxxxxx.com/gwtpage/|C49143D90DACAAD5E043EA78E4C4678F|com.xxxxxxx.xxxxx.gwt.client.server.PageController|getCustomizedPage|java.lang.String/2004016611|[Ljava.lang.String;/2600011424|rs|adw=fejj&rri=&cx|1|2|3|4|2|5|6|7|6|1|8|"

will give 3 arguments using current HttpSamplerBase#parseArguments algorithm:
- Argument 1) name=7|0|8|http://www.geniuswiki.com/gwtpage/|C49143D90DACAAD5E043EA78E4C4678F|com.edgenius.wiki.gwt.client.server.PageController|getCustomizedPage|java.lang.String/2004016611|[Ljava.lang.String;/2600011424|rs|adw ,value=fejj
- Argument 2) name=rri ,value=
- Argument 3) name=cx|1|2|3|4|2|5|6|7|6|1|8| ,value=

And this has no meaning in terms of parameter.
There is another issue related to "encode ?" option, how can we guess in RAW Data when user adds data in Text Area ?

Other question, how can we say that raw data cannot be converted ? no "=" sign ? or value without name 

That's why I wanted an ergnonomy where user would choose not to have a table parameter and would loose his data when switching to say you are reinitializing POST data ? a popup could ask him if he wants to lose his RAW data or his "Parameters" data.
What do you think of a radio button to switch + popup ?

Thanks
Regards
Philippe

Or what about adding a selex box named "Request Body type" with following choices:
- Classic  (would display parameters table)
- GWT  (would display raw data)
- REST (would display raw data text area)
- JSON (would display raw data text area)


This would be useful for HTTP request that contains only Post Body like:
- XML
- Custom format
- JSON
- GWT

Regards
Philippe

I now think we should only provide conversion to Raw format; the reverse is ill-defined.

If there is no data, then switching between Parameters and Raw should be allowed with no further user interaction.

If the Parameter data cannot be converted to Raw, then the user should be prevented from doing so:
- raise an error dialog, or
- disable the tab (if convertibility is not too expensive to track).

If the Parameter data can be converted (i.e. no names), then I think we still need to warn the user that the Parameter data will be lost.

Perhaps via a modal dialog that warns that the unselected tab will be cleared when the test plan is saved or the user switches to a different test element.

In the meantime, the user can flip between tabs to check the conversion was what they expected.

If there is data in the Raw panel, then the user should be prevented from switching (that would be easy to track).

(In reply to comment #17)
> Or what about adding a selex box named "Request Body type" with following
> choices:
> - Classic  (would display parameters table)
> - GWT  (would display raw data)
> - REST (would display raw data text area)
> - JSON (would display raw data text area)

Good idea to have more tabs if they need different treatment, but I would use the tabs to select them.

Created attachment 27583 [details]
Fix to the issue implementing what you described in your N-1 comment

Hello Sebb,
I implemented what you described in N-1 comment.
There is one thing I didn't implement because I am not sure to understand: 
- In the meantime, the user can flip between tabs to check the conversion was
what they expected.

I think implementing this means converting back and forth from Raw to Parameters so introduces many issues, but I am not sure this is what you meant.

Regards
Philippe Mouawad

(In reply to comment #20)
> Created attachment 27583 [details]
> Fix to the issue implementing what you described in your N-1 comment
> 
> Hello Sebb,
> I implemented what you described in N-1 comment.
> There is one thing I didn't implement because I am not sure to understand: 
> - In the meantime, the user can flip between tabs to check the conversion was
> what they expected.
>
> I think implementing this means converting back and forth from Raw to
> Parameters so introduces many issues, but I am not sure this is what you meant.

I was thinking that the user could flip between Parameter and Raw tabs once the raw tab had been filled from the parameter tab; however earlier I said that it should not be possible to switch from the raw tab - that's an inconsistency.

But if the raw tab still matched what the parameter data would create, then we could allow switching back and forth.

Created attachment 27600 [details]
Fix to the issue taking into account your last comment

Hello Sebb,
Last version, hope it will match what you are expecting.
Regards
Philippe Mouawad

Created attachment 27602 [details]
Fix to issue computing everything said in comments

Hello,
Last version, hope it will match what you are expecting.
Regards
Philippe Mouawad

(In reply to comment #24)
> Created attachment 27602 [details]
> Fix to issue computing everything said in comments
> 
> Hello,
> Last version, hope it will match what you are expecting.

It sort of works, however there are some issues:
- it's risky saving the selection in the field selectedTPIndex - too easy to get out of step.
- if the raw data does not match the parameter data, it's not possible to switch back - which is fine - but there's no indication why it's not allowed.
- a blank parameter list prevents unconfirmed switching to Raw mode, but there's no data to loose in that case.

(In reply to comment #25)
> (In reply to comment #24)
> > Created attachment 27602 [details]
> > Fix to issue computing everything said in comments
> > 
> > Hello,
> > Last version, hope it will match what you are expecting.
> 
> It sort of works, however there are some issues:
> - it's risky saving the selection in the field selectedTPIndex - too easy to
> get out of step.
What do you mean by get out of step ? Do you see a better solution ?
> - if the raw data does not match the parameter data, it's not possible to
> switch back - which is fine - but there's no indication why it's not allowed.
I'll fix it
> - a blank parameter list prevents unconfirmed switching to Raw mode, but
> there's no data to loose in that case.
I'll fix it


Thanks
Regards
Philippe

(In reply to comment #26)
> (In reply to comment #25)
> > (In reply to comment #24)
> > > Created attachment 27602 [details]
> > > Fix to issue computing everything said in comments
> > > 
> > > Hello,
> > > Last version, hope it will match what you are expecting.
> > 
> > It sort of works, however there are some issues:
> > - it's risky saving the selection in the field selectedTPIndex - too easy to
> > get out of step.
> What do you mean by get out of step ? 

It's a copy of postContentTabbedPane.getSelectedIndex().
Whenever code maintains a copy of something there's a possibility that a code path will fail to keep the copies in step.

> Do you see a better solution ?

It only seems to be needed to determine whether or not the tab is being changed; surely there is a way to detect this from the ChangeEvent?

Created attachment 27613 [details]
Fix that takes into account your notes

Hello Sebb,
New try taking into account your remarks.
The only way I found was to extend JTabbedPane, if you know another way.
ChangeEvent has not data that gives index.

Regards
Philippe Mouawad

(In reply to comment #28)
> Created attachment 27613 [details]
> Fix that takes into account your notes
> 
> Hello Sebb,
> New try taking into account your remarks.

Thanks.

> The only way I found was to extend JTabbedPane, if you know another way.
> ChangeEvent has not data that gives index.

Looks like extending it is the only way.

An alternative would be to use CardLayout, but that is harder to code.
It's OK as used by LDAP Sampler, but here tabs look better, so I don't want to swap.

Hello Sebb,
Thank you for your review.
Is the patch Ok for you now or you want additional modifications ?

Thank you
Regards
Philippe

(In reply to comment #30)
> Hello Sebb,
> Thank you for your review.
> Is the patch Ok for you now or you want additional modifications ?

I'm still working on testing it; if any further mods are needed I can do them myself.

Thanks for all the work you have put into this.

Created attachment 27790 [details]
Patch to issue

Fix to issue that takes into account discussion on dev list.

Regards
Philippe

Created attachment 27791 [details]
Test Plan for mirror in version 2.5.1

Plan that shows behaviour when Request Default uses parameters in version 2.5.1

Created attachment 27792 [details]
Test Plan for Mirror with patched version

Test plan that shows backward compatibility is respected and new behaviour when raw is used (i.e. ignore request defaults).

Hi,

I've got the problem that I record a raw POST request (consists of XML) and that the http sampler samples this differently than the browser and the server cannot process this.

The browser does send:
POST /myurl HTTP/1.1
....
Content-Length: 7261
Connection: Keep-Alive

<?xml version'1.0' encoding='ISO-8859-1'?><ROOT>......</ROOT>

JMeter does send:
POST /myurl HTTP/1.1
....
Content-Type: application/x-www-form-urlencoded
Connection: keep-alive
Content-Length: 7334

<?xml version'1.0' encoding='ISO-8859-1'?><ROOT>......</ROOT>

Manually removing the line PostWriter.java:297 
connection.setRequestProperty(HTTPConstants.HEADER_CONTENT_TYPE, HTTPConstants.APPLICATION_X_WWW_FORM_URLENCODED);
which is prepended by a // TODO: is this the correct default?
did not help.

In the proposed patch here I can't find the message "post_as_rawbody" which was added tot he messages.properties used anywhere in another file, so I am not sure what the effect of the drop down box "POST Body Mode" on the sampler will be.

Date: Thu Oct 20 11:17:25 2011
New Revision: 1186738

URL: http://svn.apache.org/viewvc?rev=1186738&view=rev
Log:
Bug 51861 - Improve HTTP Request GUI to better show parameters without name (GWT RPC requests for example)

Modified:
   jakarta/jmeter/trunk/src/core/org/apache/jmeter/resources/messages.properties
   jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/config/gui/HttpDefaultsGui.java
   jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/config/gui/MultipartUrlConfigGui.java
   jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
   jakarta/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
   jakarta/jmeter/trunk/xdocs/changes.xml

There's a minor problem with the functionality - the lines sent to the server should be terminated in CRLF, but are currently only terminated with LF.

Body text is supposed to be CRLF terminated.

I'm working on a fix - this is just so it's not forgotten

Perhaps add CRLF to each line when storing to the JMX file and remove when populating the GUI - that way the code does not have to do anything special

Created attachment 28019 [details]
Test case for CRLF in raw & parameters

The two output files RawCRLF1.plain and RawCRLF2.plain should be identical, but the first is missing CR before LF

Not yet clear whether the last line of a post body should always have CRLF or whether that can be omitted.

(In reply to comment #39)
> Not yet clear whether the last line of a post body should always have CRLF or
> whether that can be omitted.

The solution does not add an extra CRLF at the end; if it is required, the last entry should have a new-line after it.

URL: http://svn.apache.org/viewvc?rev=1210275&view=rev
Log:
Bug 51861 - Improve HTTP Request GUI to better show parameters without name (GWT RPC requests for example)
Fix up code so CRLF is sent after each line.

Modified:
   jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/config/gui/UrlConfigGui.java
   jmeter/trunk/xdocs/usermanual/component_reference.xml

*** Bug 44325 has been marked as a duplicate of this bug. ***

*** Bug 46178 has been marked as a duplicate of this bug. ***

This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/2553