feat/CUS-8113-Migrated the class to rest api application #242
Conversation
WalkthroughThe pull request adds a minor version bump to the project and introduces two new REST API action classes for numeric comparison operations. Both classes evaluate conditional expressions between test data values using comparison operators. Changes
Sequence DiagramsequenceDiagram
participant Executor
participant Action as ConditionalOperators/<br/>ConditionalOperatorsIf
participant Parser as Double.parseDouble()
participant Logger
Executor->>Action: execute()
Action->>Parser: Parse testData1 to double
Action->>Parser: Parse testData2 to double
Note over Action: Switch on operator<br/>(>, <, >=, <=, ==)
alt Comparison True
Action->>Logger: Log success
Action->>Action: setSuccessMessage()
Action-->>Executor: Result.SUCCESS
else Comparison False
Action->>Logger: Log failure
Action->>Action: setErrorMessage()
Action-->>Executor: Result.FAILED
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The version bump is trivial, but the two new action classes require careful review of the comparison logic implementation, operator handling, error cases, and consistency between the two similar implementations. Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 8
🧹 Nitpick comments (4)
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java (2)
23-24: Add error handling for invalid numeric inputs.
Double.valueOf()will throwNumberFormatExceptionif the test data values cannot be parsed as valid numbers. Consider wrapping these calls in a try-catch block to provide a more user-friendly error message.@Override public Result execute() throws NoSuchElementException { + try { String operatorString = operator.getValue().toString(); double a = Double.valueOf(testData1.getValue().toString()); double b = Double.valueOf(testData2.getValue().toString()); Result result = Result.SUCCESS; switch (operatorString) { // ... rest of switch cases } return result; + } catch (NumberFormatException e) { + logger.error("Invalid numeric input: " + e.getMessage()); + setErrorMessage("Invalid numeric input. Please ensure test data contains valid numbers."); + return Result.FAILED; + } }
26-92: Add a default case to handle unexpected operators.Although the
allowedValuesconstraint limits valid operators, defensive coding suggests adding a default case to handle unexpected scenarios gracefully.break; } + default: + logger.error("Unsupported operator: " + operatorString); + setErrorMessage("Unsupported operator: " + operatorString); + return Result.FAILED; + } return result;rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java (2)
26-27: Add error handling for invalid numeric inputs.Same issue as in
ConditionalOperatorsIf:Double.valueOf()can throwNumberFormatException. Consider adding try-catch block for better error handling.See the suggested fix in the
ConditionalOperatorsIf.javareview comments.
29-95: Add a default case to the switch statement.Same as in
ConditionalOperatorsIf: add a default case to handle unexpected operators defensively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rest_api_utils/pom.xml(1 hunks)rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java(1 hunks)rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java(1 hunks)
🔇 Additional comments (3)
rest_api_utils/pom.xml (1)
9-9: LGTM!The version bump from 1.0.0 to 1.0.1 is appropriate for this minor release that adds new REST API action classes.
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java (1)
13-13: Clarify the purpose of "[Latest]" prefix and relationship with ConditionalOperatorsIf.The
[Latest]prefix in the actionText and the absence ofactionTypedifferentiate this class fromConditionalOperatorsIf. Please clarify:
- Is this intended to supersede
ConditionalOperatorsIf?- Should
ConditionalOperatorsIfbe deprecated?- What is the intended use case difference between these two nearly identical classes?
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java (1)
7-7: Verify if NoSuchElementException is necessary.The execute method declares
throws NoSuchElementException, but this exception is never thrown by the method logic. If this exception is not required by the RestApiAction contract, consider removing it from the throws clause.
| import com.testsigma.sdk.ApplicationType; | ||
| import com.testsigma.sdk.RestApiAction; | ||
| import com.testsigma.sdk.Result; | ||
| import com.testsigma.sdk.WebAction; |
There was a problem hiding this comment.
Remove unused import.
WebAction is imported but never used in this class. The class extends RestApiAction, not WebAction.
-import com.testsigma.sdk.WebAction;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import com.testsigma.sdk.WebAction; |
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java
around line 6, remove the unused import "import com.testsigma.sdk.WebAction;"
since the class extends RestApiAction and never references WebAction; simply
delete that import line to eliminate the unused import warning and keep imports
minimal.
| public Result execute() throws NoSuchElementException { | ||
| String operatorString = operator.getValue().toString(); | ||
| double a = Double.valueOf(testData1.getValue().toString()); | ||
| double b = Double.valueOf(testData2.getValue().toString()); | ||
| Result result = Result.SUCCESS; | ||
| switch (operatorString) { | ||
| case ">": | ||
| if (a > b) { | ||
| logger.info("Passed " + a + " is > " + b); | ||
| setSuccessMessage("Passed " + a + " is > " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not > " + b); | ||
| setErrorMessage("Validation failed: " + a + " is not > " + b); | ||
| System.out.println("failure"); | ||
| } | ||
|
|
||
| break; | ||
| case "<": | ||
| if (a < b) { | ||
| logger.info("Passed " + a + " is < " + b); | ||
| setSuccessMessage("Passed " + a + " is < " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not < " + b); | ||
| setErrorMessage("Validation failed: " + a + " is not < " + b); | ||
| System.out.println("failure"); | ||
| } | ||
|
|
||
| break; | ||
| case ">=": | ||
| if (a >= b) { | ||
| logger.info("Passed " + a + " is >= " + b); | ||
| setSuccessMessage("Passed " + a + " is >= " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not >= " + b); | ||
| setErrorMessage("Validation failed:" + a + " is not >= " + b); | ||
| System.out.println("failure"); | ||
| } | ||
|
|
||
| break; | ||
| case "<=": | ||
| if (a <= b) { | ||
| logger.info("Passed " + a + " is <= " + b); | ||
| setSuccessMessage("Passed " + a + " is <= " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not <= " + b); | ||
| setErrorMessage("Validation failed: " + a + " is not <= " + b); | ||
| System.out.println("failure"); | ||
| } | ||
|
|
||
| break; | ||
| case "==": | ||
| if (a == b) { | ||
| logger.info("Passed " + a + " is == " + b); | ||
| setSuccessMessage("Passed " + a + " is == " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is != " + b); | ||
| setErrorMessage("Validation failed: " + a + " is != " + b); | ||
| System.out.println("failure"); | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication with ConditionalOperatorsIf.
The execute() method is nearly identical to ConditionalOperatorsIf.execute(). This violates the DRY (Don't Repeat Yourself) principle and creates maintenance burden.
Consider extracting the common comparison logic into a shared protected method in the RestApiAction base class or a utility class. Both classes could then call this shared method.
Example refactoring approach:
Create a shared utility method:
protected Result performNumericComparison(double a, double b, String operator) {
// Common logic here
}Then both classes can use:
@Override
public Result execute() throws NoSuchElementException {
String operatorString = operator.getValue().toString();
double a = Double.valueOf(testData1.getValue().toString());
double b = Double.valueOf(testData2.getValue().toString());
return performNumericComparison(a, b, operatorString);
}🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java
around lines 24-98, the execute() method duplicates nearly identical comparison
logic found in ConditionalOperatorsIf; extract that logic into a single shared
protected method (e.g., in RestApiAction or a new utility class) with signature
like performNumericComparison(double a, double b, String operator) that
evaluates the operator, sets success/error messages, logs appropriately, and
returns Result; then simplify execute() to parse operator and operands and
delegate to this shared method, removing duplicated prints and ensuring
consistent log/error messages.
| if (a > b) { | ||
| logger.info("Passed " + a + " is > " + b); | ||
| setSuccessMessage("Passed " + a + " is > " + b); | ||
| System.out.print("Success"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove System.out statements.
Same as in ConditionalOperatorsIf: remove all System.out.print/println statements. Logging is already handled by the logger.
Also applies to: 39-39, 47-47, 52-52, 60-60, 65-65, 73-73, 78-78, 86-86, 91-91
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java
around lines 34, 39, 47, 52, 60, 65, 73, 78, 86 and 91, remove all
System.out.print/System.out.println calls; replace them with appropriate logger
calls or delete them entirely since logging is already handled by the existing
logger, ensuring no System.out usage remains in the file.
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not >= " + b); | ||
| setErrorMessage("Validation failed:" + a + " is not >= " + b); |
There was a problem hiding this comment.
Fix formatting inconsistency in error message.
Same issue as in ConditionalOperatorsIf (line 61): missing space after colon in "Validation failed:".
- setErrorMessage("Validation failed:" + a + " is not >= " + b);
+ setErrorMessage("Validation failed: " + a + " is not >= " + b);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setErrorMessage("Validation failed:" + a + " is not >= " + b); | |
| setErrorMessage("Validation failed: " + a + " is not >= " + b); |
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java
around line 64, the error message string lacks a space after the colon
("Validation failed:"), causing inconsistent formatting; update the
setErrorMessage call to insert a space after the colon so it reads "Validation
failed: " before concatenating variables (match formatting used in
ConditionalOperatorsIf).
| if (a == b) { | ||
| logger.info("Passed " + a + " is == " + b); | ||
| setSuccessMessage("Passed " + a + " is == " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is != " + b); | ||
| setErrorMessage("Validation failed: " + a + " is != " + b); | ||
| System.out.println("failure"); | ||
| } |
There was a problem hiding this comment.
Critical: Avoid using == for double comparison.
Same critical issue as in ConditionalOperatorsIf: using == for double comparison is problematic due to floating-point precision.
Use tolerance-based comparison: Math.abs(a - b) < 0.0000001 instead of a == b.
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperators.java
around lines 83 to 92, the code uses the == operator to compare double values (a
== b) which is unsafe for floating‑point; replace this with a tolerance-based
check such as Math.abs(a - b) < 1e-7 (or a defined EPSILON constant) to
determine equality, update the if/else to use that condition, and keep the
logging and success/error messages unchanged (or adjust wording to indicate
approximate equality) to reflect the new comparison.
| if (a > b) { | ||
| logger.info("Passed " + a + " is > " + b); | ||
| setSuccessMessage("Passed " + a + " is > " + b); | ||
| System.out.print("Success"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove System.out statements in favor of logger.
Using System.out.print/println in production code is not a best practice. The class already has a logger that should be used instead. Additionally, there's inconsistency between print (no newline) and println, as well as capitalization ("Success" vs "failure").
- System.out.print("Success");
- System.out.println("failure");Remove all System.out statements throughout the method, as logging is already handled by the logger.
Also applies to: 34-34, 36-36, 44-44, 47-47, 49-49, 57-57, 60-60, 62-62, 70-70, 73-73, 75-75, 83-83, 86-86, 88-88
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java
around lines 31-88, several System.out.print/println calls (lines
31,34,36,44,47,49,57,60,62,70,73,75,83,86,88) should be removed and replaced
with the class logger; update each occurrence to use
logger.debug/info/warn/error as appropriate, standardize message formatting and
capitalization (e.g., "Success" -> "success" or follow project convention) and
ensure messages include context rather than relying on newlines, removing any
direct System.out usage entirely.
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is not >= " + b); | ||
| setErrorMessage("Validation failed:" + a + " is not >= " + b); |
There was a problem hiding this comment.
Fix formatting inconsistency in error message.
Missing space after colon. Should be "Validation failed: " (note the space after colon) to match other error messages in the method.
- setErrorMessage("Validation failed:" + a + " is not >= " + b);
+ setErrorMessage("Validation failed: " + a + " is not >= " + b);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setErrorMessage("Validation failed:" + a + " is not >= " + b); | |
| setErrorMessage("Validation failed: " + a + " is not >= " + b); |
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java
around line 61, the error message concatenation lacks a space after the colon;
change the string from "Validation failed:" + a + " is not >= " + b to include a
space after the colon so it reads "Validation failed: " + a + " is not >= " + b
to match the other messages' formatting.
| if (a == b) { | ||
| logger.info("Passed " + a + " is == " + b); | ||
| setSuccessMessage("Passed " + a + " is == " + b); | ||
| System.out.print("Success"); | ||
| } else { | ||
| result = Result.FAILED; | ||
| logger.warn("Validation failed: " + a + " is != " + b); | ||
| setErrorMessage("Validation failed: " + a + " is != " + b); | ||
| System.out.println("failure"); | ||
| } |
There was a problem hiding this comment.
Critical: Avoid using == for double comparison.
Using == to compare doubles is problematic due to floating-point precision issues. Use a tolerance-based comparison instead.
case "==":
- if (a == b) {
+ if (Math.abs(a - b) < 0.0000001) {
logger.info("Passed " + a + " is == " + b);
setSuccessMessage("Passed " + a + " is == " + b);
System.out.print("Success");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (a == b) { | |
| logger.info("Passed " + a + " is == " + b); | |
| setSuccessMessage("Passed " + a + " is == " + b); | |
| System.out.print("Success"); | |
| } else { | |
| result = Result.FAILED; | |
| logger.warn("Validation failed: " + a + " is != " + b); | |
| setErrorMessage("Validation failed: " + a + " is != " + b); | |
| System.out.println("failure"); | |
| } | |
| if (Math.abs(a - b) < 0.0000001) { | |
| logger.info("Passed " + a + " is == " + b); | |
| setSuccessMessage("Passed " + a + " is == " + b); | |
| System.out.print("Success"); | |
| } else { | |
| result = Result.FAILED; | |
| logger.warn("Validation failed: " + a + " is != " + b); | |
| setErrorMessage("Validation failed: " + a + " is != " + b); | |
| System.out.println("failure"); | |
| } |
🤖 Prompt for AI Agents
In
rest_api_utils/src/main/java/com/testsigma/addons/restapi/ConditionalOperatorsIf.java
around lines 80 to 89, the code uses `==` to compare doubles which is
unreliable; replace the direct equality check with a tolerance-based comparison
(e.g., use Math.abs(a - b) <= EPSILON) where EPSILON is a small constant (like
1e-9) or a configurable threshold; also handle special cases (NaN/infinite) if
relevant by checking Double.isNaN/Double.isInfinite before comparison, and
update the log and message text to reflect "approximately equal" when the
tolerance check succeeds.
Addon Name: Rest API Utils
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/3072/addons
Jira : https://testsigma.atlassian.net/browse/CUS-8113
Migrated the class to rest api application
Summary by CodeRabbit