[CUS-9364] Convert excel rows with headers into json string/file. #274
Conversation
WalkthroughAdds a new Maven module that converts a specific row from CSV/Excel files to JSON, including a core utility for file handling and parsing, platform-specific actions for Web/Android/iOS, and project configuration/resources. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Platform Action (Web/Android/iOS)
participant Utils as ExcelFileUtils
participant FS as File System / Temp Files
participant Runtime as RunTimeData
Action->>Utils: convertRowToJson(filePath, rowNumber)
alt filePath is HTTP(S)
Utils->>FS: download remote file to temp
FS-->>Utils: temp file path
end
Utils->>Utils: detect extension (csv/xls/xlsx) and parse
Utils-->>Action: return JSON string
alt format == "JSON string"
Action->>Runtime: put(variableName, jsonString)
Runtime-->>Action: ack
else format == "JSON file"
Action->>FS: write temp JSON file
FS-->>Action: json file path
Action->>Runtime: put(variableName, jsonFilePath)
Runtime-->>Action: ack
end
Action-->>Action: set SUCCESS / set FAILED on exceptions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (6)
excel_to_json_util/pom.xml (1)
36-46: JUnit milestone version and TestNG scope issue.
- JUnit Jupiter
5.8.0-M1is a pre-release milestone version. Use a stable release (e.g.,5.10.0or later).- TestNG dependency is missing
<scope>test</scope>, causing it to be bundled in the production artifact.<dependency> <groupId>org.junit.jupiter</groupId> <artifactId>junit-jupiter-api</artifactId> - <version>${junit.jupiter.version}</version> + <version>5.10.0</version> <scope>test</scope> </dependency> <dependency> <groupId>org.testng</groupId> <artifactId>testng</artifactId> - <version>6.14.3</version> + <version>7.8.0</version> + <scope>test</scope> </dependency>excel_to_json_util/src/main/java/com/testsigma/addons/ios/ExcelRowToJsonIOSAction.java (1)
12-12: Unused import:NoSuchElementException.
NoSuchElementExceptionis imported and declared in the method signature but never thrown or caught within this action. Consider removing the import and the throws clause.excel_to_json_util/src/main/java/com/testsigma/addons/android/ExcelRowToJsonAndroidAction.java (1)
39-93: Consider extracting common logic to reduce duplication.The
execute()method is nearly identical across iOS, Android, and Web actions. Consider extracting the core logic to a shared helper method or abstract base class to improve maintainability and reduce the risk of divergent bug fixes.// Example: Add a static helper in ExcelFileUtils public static void processAndStoreResult( String filePathStr, int rowNum, String varName, String format, Consumer<com.testsigma.sdk.RunTimeData> runTimeDataSetter, Consumer<String> successMessageSetter, Logger logger) throws IOException { // Common logic here }excel_to_json_util/src/main/java/com/testsigma/addons/web/ExcelRowToJsonWebAction.java (1)
18-18: Inconsistent actionText wording across platforms.The Web action uses
"...file-path in format into the runtime variable..."while iOS/Android use"...file-path into the format in runtime variable...". Consider aligning the wording for consistency across platforms.excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java (2)
63-64: Incorrect log level for error scenario.Error conditions should use
logger.erroror at minimumlogger.warn, notlogger.info. This makes it harder to filter and detect issues in logs.- logger.info("Error while accessing: " + url); - logger.debug(ExceptionUtils.getStackTrace(e)); + logger.error("Error while accessing: " + url); + logger.error(ExceptionUtils.getStackTrace(e));
178-193: Temp files not cleaned up after processing.The method relies on
deleteOnExit()for temp file cleanup, but this only occurs on JVM termination. In long-running processes or high-volume conversions, temp files accumulate on disk. Consider explicit cleanup after processing.public String convertRowToJson(String filePath, int rowNumber) throws IOException { // Convert URL to file if needed File file = urlToFileConverter(filePath); String fileName = file.getName().toLowerCase(); + boolean isTempFile = !filePath.equals(file.getAbsolutePath()); - // Determine file type and process accordingly - if (fileName.endsWith(".csv")) { - return convertCsvRowToJson(file.getAbsolutePath(), rowNumber); - } else if (fileName.endsWith(".xlsx")) { - return convertExcelRowToJson(file.getAbsolutePath(), rowNumber, true); - } else if (fileName.endsWith(".xls")) { - return convertExcelRowToJson(file.getAbsolutePath(), rowNumber, false); - } else { - throw new IOException("Unsupported file format. Supported formats: .csv, .xls, .xlsx"); + try { + // Determine file type and process accordingly + if (fileName.endsWith(".csv")) { + return convertCsvRowToJson(file.getAbsolutePath(), rowNumber); + } else if (fileName.endsWith(".xlsx")) { + return convertExcelRowToJson(file.getAbsolutePath(), rowNumber, true); + } else if (fileName.endsWith(".xls")) { + return convertExcelRowToJson(file.getAbsolutePath(), rowNumber, false); + } else { + throw new IOException("Unsupported file format. Supported formats: .csv, .xls, .xlsx"); + } + } finally { + if (isTempFile && file.exists()) { + file.delete(); + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
excel_to_json_util/pom.xml(1 hunks)excel_to_json_util/src/main/java/com/testsigma/addons/android/ExcelRowToJsonAndroidAction.java(1 hunks)excel_to_json_util/src/main/java/com/testsigma/addons/ios/ExcelRowToJsonIOSAction.java(1 hunks)excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java(1 hunks)excel_to_json_util/src/main/java/com/testsigma/addons/web/ExcelRowToJsonWebAction.java(1 hunks)excel_to_json_util/src/main/resources/testsigma-sdk.properties(1 hunks)
🔇 Additional comments (2)
excel_to_json_util/src/main/java/com/testsigma/addons/web/ExcelRowToJsonWebAction.java (1)
33-34: VerifyisRuntimeVariable = trueannotation.The
variable-namefield hasisRuntimeVariable = truein the Web action but not in iOS/Android. Ensure this difference is intentional for the SDK behavior on each platform.excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java (1)
220-221: Formula cells return formula text instead of computed value.The
FORMULAcase returns the formula string (e.g.,"=SUM(A1:A10)") rather than the evaluated result. Users typically expect the computed value. Consider usingFormulaEvaluatorto get the actual value.+ private FormulaEvaluator formulaEvaluator; // Initialize in convertExcelRowToJson + private String getCellValueAsString(Cell cell) { // ... existing cases ... case FORMULA: - return cell.getCellFormula(); + try { + CellValue cellValue = formulaEvaluator.evaluate(cell); + switch (cellValue.getCellType()) { + case STRING: + return cellValue.getStringValue(); + case NUMERIC: + double numVal = cellValue.getNumberValue(); + return numVal == (long) numVal ? String.valueOf((long) numVal) : String.valueOf(numVal); + case BOOLEAN: + return String.valueOf(cellValue.getBooleanValue()); + default: + return ""; + } + } catch (Exception e) { + return cell.getCellFormula(); // Fallback to formula string + }Please verify if returning the formula text is the intended behavior or if users expect the computed value.
excel_to_json_util/src/main/java/com/testsigma/addons/android/ExcelRowToJsonAndroidAction.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/ios/ExcelRowToJsonIOSAction.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/web/ExcelRowToJsonWebAction.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java (3)
100-104: The missing validation forrowNumber < 1was flagged in a previous review. PassingrowNumber = 0or negative values will causeIndexOutOfBoundsExceptionat line 104.
147-151: The missing validation forrowNumber < 1was flagged in a previous review. PassingrowNumber = 0would return header row data, inconsistent with documented behavior.
251-260: The missing\b(backspace) and\f(form feed) escaping was flagged in a previous review. Per RFC 8259, these control characters must be escaped in JSON strings.
🧹 Nitpick comments (4)
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java (4)
21-24: Make the logger fieldprivate final.The
loggerfield is package-private and mutable. It should beprivate finalfor proper encapsulation.- Logger logger; + private final Logger logger; + public ExcelFileUtils(Logger logger) { this.logger = logger; }
42-42: Misleading log message.The log says "s3 url" but this branch handles any HTTP/HTTPS URL, not just S3. Consider generalizing to "remote URL" or "HTTP(S) URL".
50-50: Add connection and read timeouts to prevent hanging.
FileUtils.copyURLToFilewithout timeout parameters can block indefinitely if the server doesn't respond. Use the overload with timeout parameters.- FileUtils.copyURLToFile(urlObject, tempFile); + int connectionTimeoutMs = 30_000; // 30 seconds + int readTimeoutMs = 60_000; // 60 seconds + FileUtils.copyURLToFile(urlObject, tempFile, connectionTimeoutMs, readTimeoutMs);
203-226: Consider handling ERROR cell type explicitly.Cells with errors (e.g.,
#DIV/0!,#REF!) fall through to the default case and return an empty string silently. Consider returning the error code or logging a warning so users are aware of problematic cells.+ case ERROR: + return FormulaError.forInt(cell.getErrorCellValue()).getString(); case BLANK: return ""; default: return "";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java(1 hunks)
🔇 Additional comments (1)
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java (1)
232-246: LGTM!The manual JSON construction is straightforward for this simple key-value use case. The escaping is properly delegated to
escapeJson.
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
excel_to_json_util/src/main/java/com/testsigma/addons/util/ExcelFileUtils.java
Show resolved
Hide resolved
please review this addon and publish as PUBLIC
Addon name : Excel To Json Util
Addon accont: https://jarvis.testsigma.com/ui/tenants/3072/addons
Jira: https://testsigma.atlassian.net/browse/CUS-9364
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.