[CUS-8522] Add-on to find json nodes with null values. #256
Conversation
WalkthroughThis PR adds a JSONUtilities helper and implements platform-specific JSON actions (CaptureNodesWithNullOrBlankValues, ExtractTextUsingJsonPathInJson, StoreJSONDataIntoRuntimeVariable, VerifyJsonOrder) across Android, iOS, Web, Windows, and generator targets; also bumps project version from 1.0.3 to 1.0.5. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Action (Extract/Capture/Store/Verify)
participant JSONUtil as JSONUtilities
participant Jackson as Jackson Parser
participant JsonPath as JsonPath Evaluator
participant Runtime as RunTimeData
Action->>JSONUtil: preprocessJsonString(jsonText, logger)
JSONUtil-->>Jackson: sanitized JSON
Jackson->>Jackson: parse to JsonNode
Action->>JsonPath: evaluate(jsonPath) on JsonNode
JsonPath-->>Action: matched value(s)
Action->>JSONUtil: extractValuesFromJsonPath / analyzeNodesForNullOrBlank
JSONUtil-->>Action: values / issues
alt store result
Action->>Runtime: set(key, value)
end
Action-->>Action: return SUCCESS or FAILED
sequenceDiagram
participant Verify as VerifyJsonOrder
participant JSONUtil as JSONUtilities
participant Jackson as Jackson Parser
Verify->>JSONUtil: preprocessJsonString(jsonString, logger)
JSONUtil-->>Jackson: sanitized JSON
Jackson->>Jackson: parse as array/node
Verify->>JSONUtil: extractValuesFromJsonPath(path, logger)
JSONUtil-->>Verify: values[]
alt values empty
Verify-->>Verify: return FAILED (no values)
else
Verify->>JSONUtil: verifyOrder(values, orderType, logger)
JSONUtil-->>Verify: boolean
alt true
Verify-->>Verify: return SUCCESS
else
Verify-->>Verify: return FAILED
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
JsonUtils_982187920716202948315267669918166316366/pom.xml (1)
37-41: Replace JUnit milestone version with stable release.Line 39 uses
5.8.0-M1, a pre-release milestone version from 2021. Milestone versions are not suitable for production and should be replaced with the latest stable release (currently 5.10.x).Apply this diff to update to a stable version:
- <version>5.8.0-M1</version> + <version>5.10.1</version>
🧹 Nitpick comments (1)
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/ExtractTextUsingJsonPathInJson.java (1)
47-48: Remove redundant logger parameter.Line 47 creates
JSONUtilitieswithlogger, then line 48 passesloggeragain toreadJsonData. This suggests the API design may be redundant.If
JSONUtilitiesalready has the logger from construction, the method call should be:- String output = jsonUtilities.readJsonData(jsonString, jsonPath, logger); + String output = jsonUtilities.readJsonData(jsonString, jsonPath);Verify the
JSONUtilities.readJsonDatamethod signature to confirm if the logger parameter is necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
JsonUtils_982187920716202948315267669918166316366/pom.xml(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/CaptureNodesWithNullOrBlankValues.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/ExtractTextUsingJsonPathInJson.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/StoreJSONDataIntoRuntimeVariable.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/CaptureNodesWithNullOrBlankValues.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/ExtractTextUsingJsonPathInJson.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/StoreJSONDataIntoRuntimeVariable.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/CaptureNodesWithNullOrBlankValues.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/ExtractTextUsingJsonPathInJson.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/StoreJSONDataIntoRuntimeVariable.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/CaptureNodesWithNullOrBlankValues.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/ExtractTextUsingJsonPathInJson.java(2 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/StoreJSONDataIntoRuntimeVariable.java(2 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/CaptureNodesWithNullOrBlankValues.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/ExtractTextUsingJsonPathInJson.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/StoreJSONDataIntoRuntimeVariable.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/VerifyJsonOrder.java(1 hunks)
🔇 Additional comments (2)
JsonUtils_982187920716202948315267669918166316366/pom.xml (1)
9-9: Verify version bump and dependency security posture.Line 9 bumps the version from 1.0.3 to 1.0.5, skipping 1.0.4. Additionally, several dependencies use older versions (e.g., jackson-annotations 2.13.0, json-path 2.9.0) that may have known vulnerabilities.
Confirm:
- Whether the version skip to 1.0.5 is intentional or should be 1.0.4.
- Whether all dependency versions have been checked for security advisories.
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/ExtractTextUsingJsonPathInJson.java (1)
1-81: Verify consistency across platform implementations.The PR summary mentions that similar JSON action classes are implemented across Android, iOS, Web, Windows, and generator platforms. Ensure that the logic, error handling, and API usage are consistent across all these implementations to avoid divergent behavior.
Run the following script to identify similar implementations and check for consistency:
...18166316366/src/main/java/com/testsigma/addons/android/StoreJSONDataIntoRuntimeVariable.java
Show resolved
Hide resolved
...18166316366/src/main/java/com/testsigma/addons/android/StoreJSONDataIntoRuntimeVariable.java
Show resolved
Hide resolved
...6202948315267669918166316366/src/main/java/com/testsigma/addons/android/VerifyJsonOrder.java
Outdated
Show resolved
Hide resolved
...8166316366/src/main/java/com/testsigma/addons/generators/ExtractTextUsingJsonPathInJson.java
Show resolved
Hide resolved
...8166316366/src/main/java/com/testsigma/addons/generators/ExtractTextUsingJsonPathInJson.java
Show resolved
Hide resolved
...20716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java
Show resolved
Hide resolved
...20716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java
Show resolved
Hide resolved
...669918166316366/src/main/java/com/testsigma/addons/web/StoreJSONDataIntoRuntimeVariable.java
Show resolved
Hide resolved
...20716202948315267669918166316366/src/main/java/com/testsigma/addons/web/VerifyJsonOrder.java
Outdated
Show resolved
Hide resolved
...6202948315267669918166316366/src/main/java/com/testsigma/addons/windows/VerifyJsonOrder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/VerifyJsonOrder.java (1)
32-41: Consider null safety checks.The code calls
getValue().toString()on TestData objects without null checks. While the SDK might guarantee non-null values, defensive checks could prevent potential NPEs.Additionally, the logger is passed to
preprocessJsonString(line 40) even thoughJSONUtilitiesis already constructed with it (line 38). This pattern repeats on lines 44 and 54. If the utility methods don't need the logger parameter, consider removing it for cleaner code.JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/VerifyJsonOrder.java (1)
44-51: Remove redundant JSON parse.
mapper.readTree(preprocessedJson)is invoked only to discard the resultingJsonNode, whileextractValuesFromJsonPathwill already surface invalid JSON. Dropping this extra parse trims work and clears the unused-variable warning.-import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import com.testsigma.sdk.ApplicationType;- // Parse JSON string - ObjectMapper mapper = new ObjectMapper(); - JsonNode jsonNode = mapper.readTree(preprocessedJson); - // Extract values using JSON path via utils List<Object> extractedValues = jsonUtilities.extractValuesFromJsonPath(preprocessedJson, path, logger);JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java (3)
52-52: Update the log message to reflect actual preprocessing behavior.The log message mentions "normalized whitespace," but the code no longer normalizes whitespace (the problematic
replaceAll("\\s+", " ")was correctly removed per previous review). The method now only removes specific problematic characters like non-breaking spaces and byte order marks.Apply this diff:
- logger.info("Preprocessed JSON string: removed problematic characters and normalized whitespace"); + logger.info("Preprocessed JSON string: removed problematic characters (non-breaking spaces, byte order marks, etc.)");
118-136: Add cleanup for temporary downloaded files.Temporary files created when downloading from URLs are never explicitly deleted. While the OS will eventually clean them up, they can accumulate during test execution. Consider adding
tempFile.deleteOnExit()or using try-with-resources cleanup.Apply this diff to ensure cleanup:
File tempFile = File.createTempFile("downloaded-", fileName); + tempFile.deleteOnExit(); try (InputStream in = url.openStream(); OutputStream out = new FileOutputStream(tempFile)) {
279-322: Consider consolidating sorting logic to reduce duplication.The three sorting methods share nearly identical comparison logic. You could extract a common comparator and parameterize it by direction/strategy.
Example consolidation:
private void sortAscending(List<Object> values) { Collections.sort(values, createComparator(1, false)); } private void sortDescending(List<Object> values) { Collections.sort(values, createComparator(-1, false)); } private void sortAlphabetical(List<Object> values) { Collections.sort(values, createComparator(1, true)); } private Comparator<Object> createComparator(int direction, boolean caseInsensitive) { return new Comparator<Object>() { @Override public int compare(Object o1, Object o2) { int result; if (o1 instanceof Number && o2 instanceof Number) { result = Double.compare(((Number) o1).doubleValue(), ((Number) o2).doubleValue()); } else { String s1 = caseInsensitive ? o1.toString().toLowerCase() : o1.toString(); String s2 = caseInsensitive ? o2.toString().toLowerCase() : o2.toString(); result = s1.compareTo(s2); } return result * direction; } }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/VerifyJsonOrder.java(1 hunks)JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/windows/VerifyJsonOrder.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/android/VerifyJsonOrder.java
🔇 Additional comments (14)
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/ios/VerifyJsonOrder.java (4)
14-18: LGTM!The class declaration and annotations are well-structured. The action text clearly defines the NLP syntax, and the description is informative.
20-27: LGTM!The field declarations are correct, and the
allowedValuesconstraint onorderTypeeffectively prevents invalid inputs.
43-65: Good defensive programming and correct implementation.The empty value check (lines 46-51) is excellent defensive programming. More importantly, this implementation correctly allows JSONPath extraction to work on any JSON structure (not just arrays), which addresses the concern from the previous review.
67-72: Exception handling is acceptable for an action class.The broad
catch (Exception e)is generally acceptable in action execution contexts where any failure should return a FAILED result. The error message includes exception details, which aids debugging.JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/generators/VerifyJsonOrder.java (3)
14-27: Well-structured action definition.The class structure, annotations, and field declarations are correct. The
allowedValuesconstraint onorderTypeproperly restricts input to valid order types.
30-74: Logic and error handling look solid.The execute method correctly:
- Extracts and validates inputs
- Preprocesses JSON and extracts values via JSONPath
- Handles empty results appropriately
- Verifies order and returns clear success/failure results
- Includes exception handling with descriptive error messages
The removal of the array-only guard (mentioned in past reviews) allows this action to work correctly with JSONPath extraction from any JSON structure.
38-55: Verify if logger parameter is necessary in utility method calls.
JSONUtilitiesis instantiated withloggerat line 38, but the sameloggeris explicitly passed again to methods at lines 40, 45, and 55. This pattern may be redundant ifJSONUtilitiesmethods use the instance logger.Please verify the
JSONUtilitiesclass implementation to confirm whether the logger parameter is necessary or if the methods can use the instance logger:JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/web/VerifyJsonOrder.java (1)
28-71: LGTM! Clean implementation of JSON order verification.The implementation correctly delegates to
JSONUtilitiesfor preprocessing, extraction, and order verification. The error handling is appropriate, and the previous concern about requiring root-level arrays has been properly addressed by usingextractValuesFromJsonPath, which handles both arrays and objects at any nesting level.JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java (6)
67-100: LGTM! Robust JSON path extraction with proper error handling.The method correctly preprocesses the JSON string, validates inputs, and provides clear error messages for different failure scenarios.
200-226: LGTM! Correctly handles both list and scalar JSONPath results.The previous issue with
ClassCastExceptionwhen a JSONPath resolves to a scalar value has been properly addressed. The code now checks the result type and handles both collections and single values appropriately.
236-274: LGTM! Clear order verification logic.The method correctly handles edge cases (empty/single-element lists) and provides appropriate logging for debugging.
332-367: LGTM! Comprehensive null/blank value detection.The method appropriately handles both array and object inputs, delegating to the recursive field finder for detailed analysis.
387-390: Verify that trimming whitespace before blank check is intentional.Line 389 uses
fieldValue.asText().trim().isEmpty(), which treats strings containing only whitespace (e.g.," ") as blank. If the distinction between empty strings ("") and whitespace-only strings matters for your use case, you may want to checkisEmpty()withouttrim()and handle whitespace-only strings separately.The current behavior is:
""→ blank ✓" "→ blank ✓ (after trim)"hello"→ not blank ✓Is treating whitespace-only strings as blank the intended behavior?
449-470: LGTM! Simple and effective data transfer objects.The
NodeIssueandFieldIssueclasses provide a clear structure for representing analysis results.
...2948315267669918166316366/src/main/java/com/testsigma/addons/generators/VerifyJsonOrder.java
Show resolved
Hide resolved
| for (NodeIssue issue : issues) { | ||
| // Display node index as 1-based for user-friendly output | ||
| int displayIndex = issue.nodeIndex + 1; | ||
| output.append("Node ").append(displayIndex).append(" has the following issues:\n"); | ||
| for (FieldIssue fieldIssue : issue.fieldIssues) { | ||
| result.append(fieldIssue.fieldPath).append(","); | ||
| output.append(" - Field '").append(fieldIssue.fieldPath) | ||
| .append("' has ").append(fieldIssue.issueType).append(" value\n"); | ||
| } | ||
| output.append("\n"); | ||
| // remove trailing comma from result | ||
| if (result.length() > 0 && result.charAt(result.length() - 1) == ',') { | ||
| result.setLength(result.length() - 1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix comma handling in result formatting.
The trailing comma removal (lines 436-438) occurs inside the outer loop, which means commas are removed after each node's fields but not added between nodes. This produces inconsistent output like "field1,field2field3,field4" when field2 is from node 1 and field3 is from node 2.
If the result should be a comma-separated list of ALL field paths across all nodes, move the comma removal outside the loop:
for (NodeIssue issue : issues) {
// Display node index as 1-based for user-friendly output
int displayIndex = issue.nodeIndex + 1;
output.append("Node ").append(displayIndex).append(" has the following issues:\n");
for (FieldIssue fieldIssue : issue.fieldIssues) {
result.append(fieldIssue.fieldPath).append(",");
output.append(" - Field '").append(fieldIssue.fieldPath)
.append("' has ").append(fieldIssue.issueType).append(" value\n");
}
output.append("\n");
- // remove trailing comma from result
- if (result.length() > 0 && result.charAt(result.length() - 1) == ',') {
- result.setLength(result.length() - 1);
- }
}
+ // remove trailing comma from result
+ if (result.length() > 0 && result.charAt(result.length() - 1) == ',') {
+ result.setLength(result.length() - 1);
+ }
+
logger.info("Formatted output for " + issues.size() + " node issue(s)");🤖 Prompt for AI Agents
In
JsonUtils_982187920716202948315267669918166316366/src/main/java/com/testsigma/addons/utils/JSONUtilities.java
around lines 425-439, the trailing-comma trim is currently inside the outer loop
which strips the comma after each node and prevents commas between nodes; remove
the block that checks and trims the trailing comma from inside the for
(NodeIssue issue : issues) loop and instead perform a single trailing-comma
removal after the loop completes so result becomes a single comma-separated list
of all field paths across all nodes.
please review this addon and publish as PUBLIC
Addon name : json utils
Addon accont: https://jarvis.testsigma.com/ui/tenants/2187/addons
Jira: https://testsigma.atlassian.net/browse/CUS-8522
fix
Summary by CodeRabbit
New Features
Chores