FSchumacher commented on code in PR #5811:
URL: https://github.com/apache/jmeter/pull/5811#discussion_r1173586470
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -316,9 +324,11 @@ public static Map<String, String[]> getQueryMap(String
query) {
System.arraycopy(known, 0, tmp, 0, known.length);
known = tmp;
}
- map.put(name, known);
+ if (!noNameAndNoValue) {
Review Comment:
Double negations are hard to read.
Perhaps we could use `validNameAndValue` with a default of `true` and set it
to `false`, if `name` or `value` are missing.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -316,9 +324,11 @@ public static Map<String, String[]> getQueryMap(String
query) {
System.arraycopy(known, 0, tmp, 0, known.length);
known = tmp;
}
- map.put(name, known);
+ if (!noNameAndNoValue) {
+ map.put(name, known);
+ }
}
-
+ System.out.println("i: " + i + " map size: " + map.size());
Review Comment:
Probably not needed.
##########
src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTPTest.java:
##########
@@ -21,6 +21,7 @@
import org.apache.commons.lang3.StringUtils;
import org.junit.Assert;
Review Comment:
We could change all assertions to the newer jupiter API.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -285,9 +285,17 @@ public static Map<String, String[]> getQueryMap(String
query) {
Map<String, String[]> map = new HashMap<>();
String[] params = query.split(PARAM_CONCATENATE);
+ int i = 0;
for (String param : params) {
+ i++;
String[] paramSplit = param.split("=");
- String name = decodeQuery(paramSplit[0]);
+ String name = "";
Review Comment:
Is this OK? We use `name` further down to combine key/value-pairs with the
same key.
In the old implementation we would not get to the point, where name is not
initialized, now, we would get there with a default value of `""`.
Maybe use a `null` value of `name` to mean *no valid key* found and use
that to deduce the bool `noNameAndNoValue` later (see my other comment about
double negations).
##########
src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTPTest.java:
##########
@@ -193,4 +194,29 @@ public void testGetQueryMapSoapHack() {
Assert.assertEquals(query, param1.getValue()[0]);
Assert.assertTrue(StringUtils.isBlank(param1.getKey()));
}
+
+ @Test
Review Comment:
Here a parametrized test could perhaps be used. The test cases are quite
similar.
##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/visualizers/RequestViewHTTP.java:
##########
@@ -285,9 +285,17 @@ public static Map<String, String[]> getQueryMap(String
query) {
Map<String, String[]> map = new HashMap<>();
String[] params = query.split(PARAM_CONCATENATE);
+ int i = 0;
for (String param : params) {
+ i++;
Review Comment:
Not needed apart from the println, right?
--
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-unsubscr...@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
