You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by GitBox <gi...@apache.org> on 2021/03/12 21:24:36 UTC
[GitHub] [jmeter] FSchumacher opened a new pull request #651: Don't assume we have correct JSON at edit time
FSchumacher opened a new pull request #651:
URL: https://github.com/apache/jmeter/pull/651
## Description
Allow the usage of JMeter variables in all user definable parameters for GraphQL Sampler.
## Motivation and Context
When JMeter variables are used inside variable or query blocks for the
GraphQL Sampler, we cannot assume, that we still have valid JSON as
long those variables are not rendered. The rendering of the variables
is done at a later time in the life cycle. Therefore, we have to do
our best to render JSON without actually checking the validity.
Bugzilla Id: 65108
## How Has This Been Tested?
Test case has been added
## Screenshots (if appropriate):
## Types of changes
<!--- What types of changes does your code introduce? Delete as appropriate -->
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #651:
URL: https://github.com/apache/jmeter/pull/651#issuecomment-1001702544
Superceeded by #660
--
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-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r602893911
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Thanks for the pointers, @FSchumacher !
I'll experiment somethings in that direction. I'll have some free time during my coming international travel and (self-)quarantine. ;-) Stay safe!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r604141868
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
I took a look deeper into this and all my tries where unsuccessful, as they would have needed more changes, than I would liked to have. Good luck on your experiments.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r612353542
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Hi @FSchumacher / @vlsi ,
I found "a" solution in PR #660 . Would you mind reviewing the PR?
Thanks,
Woonsan
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi closed pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi closed pull request #651:
URL: https://github.com/apache/jmeter/pull/651
--
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-unsubscribe@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593795217
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Yes, it is probably better to move the logic of generating the JSON to a phase when the variables are already evaluated. Normally this would be done in the `sampler` method. In the GraphQL Sampler case that means more work, as it is more like a tool to fill a normal HTTP Sampler.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r594006487
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Thanks, @FSchumacher and @vlsi !
I didn't think about the variables in the http params to be resolved in samplings at the moment. Now I understand the problem from your insightful discussions above.
As @FSchumacher mentioned, it sounds like simpler with overriding with a wrapper, after variables are resolved _somehow_, if possible.
Regards, woonsan
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593798541
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
But that might be feasible, to add a wrapper inside an overridden `createTestElement()` method in `GraphQLHTTPSampler`. The wrapper could use the `sample()` method to construct a valid JSON, without having to worry about invalid JSON syntax, as the variables have already been replaced.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r597106256
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
I don't think, that we should add code to `HttpSamplerBase` as it doesn't have anything to do with the GraphQL stuff (other than being used by the *GraphQlSampler*. I think we should create a new class (we could call it `GraphQlSampler` :) ) that extends `HttpSamplerProxy` and overrides the `sample` method. The new class could be injected by overriding `HttpTestSampleGui#createTestElement()` in `GraphQLHTTPSampler`.
Alternatively we could try to override the `modifyTestElement` method and do the JSON conversion in there, when the test element is being prepared with the real data.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593793265
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Oh, I probably need to check how the sampler works, however then it becomes awkward. Currently, you suggest disabling the escape for variable content, so if the end-user types something like `Hello, "world" :}` then it would produce invalid JSON with hard to understand failures.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r596518987
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
@FSchumacher , at a glance, a quick solution can be to move the `Arguments` generation logic from `GraphQLUrlConfigGui` to `HttpSamplerBase#getArgument()`. GraphQL query/mutation and variables can still be stored during the configuration, but the http arguments are being generated in `HttpSamplerBase#getArgument()` - in case the arguments are empty and graphql params are set.
Am I in the right track?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593794397
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Just for the reference, here's how you generate raw JSON with JsonWriter:
```java
private static final JsonFactory factory = JsonFactory.builder().build();
public static String toPostBodyString(final GraphQLRequestParams params) {
StringWriter sw = new StringWriter();
try (JsonGenerator gen = factory.createGenerator(sw)) {
gen.writeStartObject();
gen.writeStringField(OPERATION_NAME_FIELD, nullIfBlank(params.getOperationName()));
String variables = params.getVariables();
gen.writeFieldName(VARIABLES_FIELD);
gen.writeRawValue(variables.trim());
gen.writeStringField(QUERY_FIELD, nullIfBlank(params.getQuery()));
gen.writeEndObject();
} catch (IOException e) {
throw new IllegalStateException("Can't write params to JSON", e);
}
return sw.toString();
}
private static String nullIfBlank(String value) {
if (value == null) {
return null;
}
value = value.trim();
return value.isEmpty() ? null : value;
}
```
However, I still believe that is the wrong direction. It fixes one issue and creates another :((
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r603261987
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Hi @FSchumacher ,
I think the simplest solution can be the following:
- Keep the current way to generate/parse to/from JSON string as a whole.
- However, do not parse the variables string to a json at all (as the string is not purely json any more).
- So, just make `GraphQLRequestParamUtils.variablesToGetParamValue(String)` generate KV style encoded JSON-like string.
Then I think the problem will be solved only by improving `GraphQLRequestParamUtils.variablesToGetParamValue(String)`.
Regards,
Woonsan
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on pull request #651:
URL: https://github.com/apache/jmeter/pull/651#issuecomment-798427591
@woonsan What do you think of this change?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] woonsan commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
woonsan commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r603261987
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Hi @FSchumacher ,
I think the simplest solution can be the following:
- Keep the current way to generate/parse to/from JSON string as a whole.
- However, do not parse the variables string to a json at all (as the string is not purely json any more).
- So, just make `GraphQLRequestParamUtils.variablesToGetParamValue(String)` generate KV style encoded JSON-like string.
Then I think the problem will be solved only by improving `GraphQLRequestParamUtils.variablesToGetParamValue(String)`.
Regards,
Woonsan
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593773122
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Have you considered `com.fasterxml.jackson.core.JsonGenerator`? Would it be better here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593794397
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Just for the reference, here's how you generate raw JSON with JsonGenerator:
```java
private static final JsonFactory factory = JsonFactory.builder().build();
public static String toPostBodyString(final GraphQLRequestParams params) {
StringWriter sw = new StringWriter();
try (JsonGenerator gen = factory.createGenerator(sw)) {
gen.writeStartObject();
gen.writeStringField(OPERATION_NAME_FIELD, nullIfBlank(params.getOperationName()));
String variables = params.getVariables();
gen.writeFieldName(VARIABLES_FIELD);
gen.writeRawValue(variables.trim());
gen.writeStringField(QUERY_FIELD, nullIfBlank(params.getQuery()));
gen.writeEndObject();
} catch (IOException e) {
throw new IllegalStateException("Can't write params to JSON", e);
}
return sw.toString();
}
private static String nullIfBlank(String value) {
if (value == null) {
return null;
}
value = value.trim();
return value.isEmpty() ? null : value;
}
```
However, I still believe that is the wrong direction. It fixes one issue and creates another :((
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593780561
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
I don't think, that it will happily generate invalid JSON formatted Strings (my shallow testing and searching has not been successful here).
The use case I try to enable is to generate a JSON structure like
```js
{ "id": ${my_var} }
```
which looks like JSON, but is not. Note the *missing* quotes around the JMeter var. This is needed, when `my_var` contains a number value. (Bugzilla Id: 65108)
At *runtime* JMeter will replace the var to generate a valid JSON structure `{ "id": 42 }` when `my_var` has the value `42`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] FSchumacher commented on a change in pull request #651: Don't assume we have correct JSON at edit time
Posted by GitBox <gi...@apache.org>.
FSchumacher commented on a change in pull request #651:
URL: https://github.com/apache/jmeter/pull/651#discussion_r593795217
##########
File path: src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,27 +76,32 @@ public static boolean isGraphQLContentType(final String contentType) {
* @throws RuntimeException if JSON serialization fails for some reason due to any runtime environment issues
*/
public static String toPostBodyString(final GraphQLRequestParams params) {
- final ObjectMapper mapper = new ObjectMapper();
- final ObjectNode postBodyJson = mapper.createObjectNode();
- postBodyJson.set(OPERATION_NAME_FIELD,
- JsonNodeFactory.instance.textNode(StringUtils.trimToNull(params.getOperationName())));
-
+ final StringBuilder result = new StringBuilder();
Review comment:
Yes, it is probably better to move the logic of generating the JSON to a phase when the variables are already evaluated. Normally this would be done in the `sample` method. In the GraphQL Sampler case that means more work, as it is more like a tool to fill a normal HTTP Sampler.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org