gmunozfe commented on code in PR #3696:
URL: 
https://github.com/apache/incubator-kie-kogito-runtimes/pull/3696#discussion_r1792279659


##########
kogito-serverless-workflow/kogito-jsonpath-expression/src/main/java/org/kie/kogito/expr/jsonpath/JsonPathExpression.java:
##########
@@ -61,24 +63,28 @@ private Configuration getConfiguration(KogitoProcessContext 
context) {
                 .build();
     }
 
+    private static boolean isContextAware(JsonNode context, Map<String, 
JsonNode> additionalVars) {
+        return !additionalVars.isEmpty() && context instanceof ObjectNode;
+    }
+
     private <T> T eval(JsonNode context, Class<T> returnClass, 
KogitoProcessContext processInfo) {
-        try (JsonNodeContext jsonNode = JsonNodeContext.from(context, 
processInfo)) {
-            Configuration jsonPathConfig = getConfiguration(processInfo);
-            DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(jsonNode.getNode());
-            if (String.class.isAssignableFrom(returnClass)) {
-                StringBuilder sb = new StringBuilder();
-                // valid json path is $. or $[
-                for (String part : expr.split("((?=\\$\\.|\\$\\[))")) {
-                    JsonNode partResult = parsedContext.read(part, 
JsonNode.class);
-                    sb.append(partResult.isTextual() ? partResult.asText() : 
partResult.toPrettyString());
-                }
-                return (T) sb.toString();
-            } else {
-                Object result = parsedContext.read(expr);
-                return Boolean.class.isAssignableFrom(returnClass) && result 
instanceof ArrayNode ? (T) Boolean.valueOf(!((ArrayNode) result).isEmpty())
-                        : 
JsonObjectUtils.convertValue(jsonPathConfig.mappingProvider().map(result, 
returnClass, jsonPathConfig), returnClass);
+
+        Configuration jsonPathConfig = getConfiguration(processInfo);
+        DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(context);
+        if (String.class.isAssignableFrom(returnClass)) {
+            StringBuilder sb = new StringBuilder();
+            // valid json path is $. or $[

Review Comment:
   In a `Foreach` , do we have to consider that the `outputCollection` may have 
different nested levels or just one? 
   Because if it's possible to have more than one nested level, there are some 
edge cases like:
   - recursive descent `$..id`, where id is at different levels of the json
   - wildcards with recursive descent: ` $..id[*].name`
   
   that won't work with that split regex.
   
   Let me put an example, suppose this data structure:
   ```json
   {
     "orders": [
       {
         "orderNumber": "1234",
         "completed": true,
         "details": {
           "items": [
             {
               "product": "Laptop",
               "quantity": 1,
               "price": 1200,
               "warranty": "2 years"
             },
             {
               "product": "Mouse",
               "quantity": 2,
               "price": 20
             }
           ],
           "shippingAddress": {
             "street": "Pez 12",
             "city": "Madrid",
             "postalCode": "28001"
           }
         }
       },
       {
         "orderNumber": "5678",
         "completed": true,
         "details": {
           "items": [
             {
               "product": "Tablet",
               "quantity": 1,
               "price": 600,
               "warranty": "1 year"
             },
             {
               "product": "T-shirt",
               "quantity": 3,
               "price": 15,
               "size": "L"
             }
           ],
           "shippingAddress": {
             "street": "Gato 13",
             "city": "Barcelona",
             "postalCode": "08005"
           }
         }
       },
       {
         "orderNumber": "9910",
         "completed": false,
         "details": {
           "items": [
             {
               "product": "Monitor",
               "quantity": 2,
               "price": 300,
               "warranty": "3 years"
             },
             {
               "product": "Keyboard",
               "quantity": 1,
               "price": 50
             }
           ],
           "shippingAddress": {
             "street": "Guadalquivir 101",
             "city": "Sevilla",
             "postalCode": "41007"
           }
         }
       }
     ]
   }
   ```
   
   In that example, we could extract in the `outputCollection` those orders 
where `completed` is `true` (same as it does in the specification). And then, 
in the `forEach`  we want to access different nested elements using 
**_recursive descent_** operator:
   
   ```
   $.orders..product
   $.orders..warranty
   $.orders..size
   ```
   
   without specifying the explicit path 
   `$.orders[*].details.items[*].size `
   but using recursive descent ($..), to easily collect data from all 
occurrences of a field, no matter where it's nested, though if it's not 
supported, I guess we can access with the explicit path, wdyt?
   
   
   



##########
kogito-serverless-workflow/kogito-jsonpath-expression/src/main/java/org/kie/kogito/expr/jsonpath/JsonPathExpression.java:
##########
@@ -61,24 +63,28 @@ private Configuration getConfiguration(KogitoProcessContext 
context) {
                 .build();
     }
 
+    private static boolean isContextAware(JsonNode context, Map<String, 
JsonNode> additionalVars) {
+        return !additionalVars.isEmpty() && context instanceof ObjectNode;
+    }
+
     private <T> T eval(JsonNode context, Class<T> returnClass, 
KogitoProcessContext processInfo) {
-        try (JsonNodeContext jsonNode = JsonNodeContext.from(context, 
processInfo)) {
-            Configuration jsonPathConfig = getConfiguration(processInfo);
-            DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(jsonNode.getNode());
-            if (String.class.isAssignableFrom(returnClass)) {
-                StringBuilder sb = new StringBuilder();
-                // valid json path is $. or $[
-                for (String part : expr.split("((?=\\$\\.|\\$\\[))")) {
-                    JsonNode partResult = parsedContext.read(part, 
JsonNode.class);
-                    sb.append(partResult.isTextual() ? partResult.asText() : 
partResult.toPrettyString());
-                }
-                return (T) sb.toString();
-            } else {
-                Object result = parsedContext.read(expr);
-                return Boolean.class.isAssignableFrom(returnClass) && result 
instanceof ArrayNode ? (T) Boolean.valueOf(!((ArrayNode) result).isEmpty())
-                        : 
JsonObjectUtils.convertValue(jsonPathConfig.mappingProvider().map(result, 
returnClass, jsonPathConfig), returnClass);
+
+        Configuration jsonPathConfig = getConfiguration(processInfo);
+        DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(context);
+        if (String.class.isAssignableFrom(returnClass)) {
+            StringBuilder sb = new StringBuilder();
+            // valid json path is $. or $[
+            for (String part : expr.split("((?=\\$\\.|\\$\\[))")) {
+                JsonNode partResult = parsedContext.read(part, JsonNode.class);
+                sb.append(partResult.isTextual() ? partResult.asText() : 
partResult.toPrettyString());

Review Comment:
   As the sb is appending (potentially) textual and non-textual parts, here 
only the non-textual are being formatted with `toPrettyString`, so in some 
cases we may have a final sb containing formatted and non-formatted parts, wdyt?



##########
kogito-serverless-workflow/kogito-jsonpath-expression/src/main/java/org/kie/kogito/expr/jsonpath/JsonPathExpression.java:
##########
@@ -61,24 +63,28 @@ private Configuration getConfiguration(KogitoProcessContext 
context) {
                 .build();
     }
 
+    private static boolean isContextAware(JsonNode context, Map<String, 
JsonNode> additionalVars) {
+        return !additionalVars.isEmpty() && context instanceof ObjectNode;
+    }
+
     private <T> T eval(JsonNode context, Class<T> returnClass, 
KogitoProcessContext processInfo) {
-        try (JsonNodeContext jsonNode = JsonNodeContext.from(context, 
processInfo)) {
-            Configuration jsonPathConfig = getConfiguration(processInfo);
-            DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(jsonNode.getNode());
-            if (String.class.isAssignableFrom(returnClass)) {
-                StringBuilder sb = new StringBuilder();
-                // valid json path is $. or $[
-                for (String part : expr.split("((?=\\$\\.|\\$\\[))")) {
-                    JsonNode partResult = parsedContext.read(part, 
JsonNode.class);
-                    sb.append(partResult.isTextual() ? partResult.asText() : 
partResult.toPrettyString());
-                }
-                return (T) sb.toString();
-            } else {
-                Object result = parsedContext.read(expr);
-                return Boolean.class.isAssignableFrom(returnClass) && result 
instanceof ArrayNode ? (T) Boolean.valueOf(!((ArrayNode) result).isEmpty())
-                        : 
JsonObjectUtils.convertValue(jsonPathConfig.mappingProvider().map(result, 
returnClass, jsonPathConfig), returnClass);
+
+        Configuration jsonPathConfig = getConfiguration(processInfo);
+        DocumentContext parsedContext = 
JsonPath.using(jsonPathConfig).parse(context);
+        if (String.class.isAssignableFrom(returnClass)) {
+            StringBuilder sb = new StringBuilder();
+            // valid json path is $. or $[
+            for (String part : expr.split("((?=\\$\\.|\\$\\[))")) {
+                JsonNode partResult = parsedContext.read(part, JsonNode.class);

Review Comment:
   how does it handle an exception (PathNotFoundException) in this method? 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to