feat: Add checkbox to enable/disable params in HTTP Sampler #6291
Conversation
Added a new "isEnabledFromGui" property in HTTPArgument used to determine whether to include the argument in the request or not. This property is set by the user for each HTTP parameter by clicking on the "Enable" column put on the very left of each parameter row. Fixes apache#5466
|
Hi @vlsi could you please help me on that? |
|
@vlsi I've also ran on my machine the task |
| @@ -30,6 +30,9 @@ import org.apiguardian.api.API | |||
| public abstract class HTTPArgumentSchema : ArgumentSchema() { | |||
| public companion object INSTANCE : HTTPArgumentSchema() | |||
|
|
|||
| public val enabledFromGui: BooleanPropertyDescriptor<HTTPArgumentSchema> | |||
| by boolean("HTTPArgument.enabled_from_gui") | |||
There was a problem hiding this comment.
Naming is hard, however, I am not sure we want stressing "from gui" part.
This is more like a generic enable/disable feature rather than "enabled in gui".
|
@pochopsp, have you tried using the default |
This reverts commit 45c45e7.
|
@vlsi you were right, there is no need for a new property in the |
vlsi
force-pushed
the
#5466-HTTP-disable-params
branch
4 times, most recently
from
facce27 to
3a066b8
Compare
| * | ||
| * @param name the name of the parameter | ||
| * @param value the value of the parameter | ||
| * @param alreadyEncoded true if the name and value is already encoded, in which case they are decoded before storage. | ||
| * @param contentEncoding the encoding used for the parameter value | ||
| */ | ||
| public HTTPArgument(String name, String value, boolean alreadyEncoded, String contentEncoding) { | ||
| setEnabled(true); |
There was a problem hiding this comment.
Is this really needed? enabled is the default state, so I am not sure what is the reason of adding it here
| @@ -299,7 +296,7 @@ public void setHeaders(URLConnection connection, HTTPSamplerBase sampler) throws | |||
|
|
|||
| // Just append all the parameter values, and use that as the post body | |||
| StringBuilder postBodyBuffer = new StringBuilder(); | |||
| for (JMeterProperty jMeterProperty : sampler.getArguments()) { | |||
| for (JMeterProperty jMeterProperty : sampler.getArguments().getEnabledArguments()) { | |||
There was a problem hiding this comment.
It looks like we should not filter "empty" arguments from here.
Frankly, I do not understand the reason why "empty named" (.isSkippable(...)) arguments are included only in certain cases.
vlsi
force-pushed
the
#5466-HTTP-disable-params
branch
3 times, most recently
from
c8b378f to
8b183d0
Compare
…ll over the place
vlsi
force-pushed
the
#5466-HTTP-disable-params
branch
from
8b183d0 to
bbce13d
Compare
|
Looks like the regression tests pass, however, we should add new tests for the disabled arguments. |
Description
This pull request adds a new column in the HTTP Sampler parameters table which allows the user to enable or disable the http parameter.
To implement this feature, a new "isEnabledFromGui" property has been added in HTTPArgument and it has been used in several places (HTTPHC4Impl, HTTPSamplerBase, PutWriter etc.) to determine whether to include the argument in the request or not.
To enable the property setting by the user a new column has been added in HTTPArgumentsPanel.
The HTTPArgumentSchema has been adjusted to include this new field too.
Tests file and localization files has been adjusted to be compliant with this new attribute.
Motivation and Context
I think this feature could be really useful, because sometimes I and other people I know wished JMeter had it while doing some tests.
Fixes #5466
How Has This Been Tested?
I have successfully built the project by using
./gradlew buildand all tests ran without issues.My testing environment is Windows 10, Intel I5 6th gen, 16GB Ram DD4, Oracle JDK 17.
Screenshots (if appropriate):
Types of changes
Checklist: