vlsi commented on a change in pull request #660:
URL: https://github.com/apache/jmeter/pull/660#discussion_r750056596



##########
File path: 
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/GraphQLRequestParamUtils.java
##########
@@ -79,28 +81,27 @@ 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())));
-
-        if (StringUtils.isNotBlank(params.getVariables())) {
-            try {
-                final ObjectNode variablesJson = 
mapper.readValue(params.getVariables(), ObjectNode.class);
-                postBodyJson.set(VARIABLES_FIELD, variablesJson);
-            } catch (JsonProcessingException e) {
-                log.error("Ignoring the GraphQL query variables content due to 
the syntax error: {}",
-                        e.getLocalizedMessage());
+        final JsonFactory jsonFactory = new JsonFactory();
+        final StringWriter writer = new StringWriter(1024);

Review comment:
       The default looks odd.
   I would go with dropping the default, or you could have better estimate 
based on the actual length of `params.getVariables()`, `params.getQuery()`, 
etc, etc.
   
   The problem with hard-coding constants is that no one knows the reasons for 
1024 to be used here, and no one would know when to revise the constant :-/




-- 
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


Reply via email to