feat/CUS-11720-Shorten temp filenames for Excel downloads from URLs#371
Merged
akhil-testsigma merged 1 commit intoApr 3, 2026
Conversation
📝 WalkthroughWalkthroughThis PR updates the ExcelActions_cloud module (version 1.0.22) by introducing a centralized URL-to-temp-file download utility, adding three new Excel verification web actions, and refactoring six existing web actions to use the shared download utility instead of duplicating download logic. Changes
Sequence DiagramsequenceDiagram
participant User
participant CompareAction as CompareExcelFiles<br/>AndLogDifferences
participant FileResolver as File Resolution<br/>(Local/URL)
participant ExcelCellUtils as ExcelCellUtils
participant WorkbookFactory as WorkbookFactory
participant Workbook as Excel Workbook
participant Logger as Logger
User->>CompareAction: execute() with file-path-1, file-path-2
CompareAction->>FileResolver: Resolve file path 1 & 2<br/>(Local path or HTTP/HTTPS URL)
alt File Path 1 is HTTP/HTTPS
FileResolver->>ExcelCellUtils: downloadUrlToTempFile(url1)
ExcelCellUtils->>ExcelCellUtils: Create temp file,<br/>download stream
ExcelCellUtils-->>FileResolver: Return temp File
else File Path 1 is Local
FileResolver-->>FileResolver: Validate local file exists
end
alt File Path 2 is HTTP/HTTPS
FileResolver->>ExcelCellUtils: downloadUrlToTempFile(url2)
ExcelCellUtils-->>FileResolver: Return temp File
else File Path 2 is Local
FileResolver-->>FileResolver: Validate local file exists
end
FileResolver-->>CompareAction: Return File objects
CompareAction->>WorkbookFactory: Load both Excel files
WorkbookFactory->>Workbook: Create workbook instances
Workbook-->>CompareAction: Workbook 1 & 2
loop For each sheet pair (max sheet count)
CompareAction->>Workbook: Get sheet by index
alt Sheet exists in both
loop For each cell in sheet
CompareAction->>ExcelCellUtils: getCellValueAsString(sheet, row, col)
ExcelCellUtils->>Workbook: Extract cell value
Workbook-->>ExcelCellUtils: Cell content
ExcelCellUtils-->>CompareAction: String value
CompareAction->>CompareAction: Compare values
alt Values differ
CompareAction->>Logger: Log difference warning<br/>(sheet, row, col, values)
CompareAction->>CompareAction: Increment totalDifferences
end
end
else Sheet missing in one workbook
CompareAction->>Logger: Log sheet mismatch
CompareAction->>CompareAction: Increment totalDifferences
end
end
alt totalDifferences == 0
CompareAction->>CompareAction: Set success message
CompareAction-->>User: Result.SUCCESS
else totalDifferences > 0
CompareAction->>CompareAction: Set HTML error message<br/>with differences & subset
CompareAction-->>User: Result.FAILED
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
🧹 Nitpick comments (3)
excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java (2)
48-49: UseWorkbookFactoryinstead ofXSSFWorkbookfor broader format support.
XSSFWorkbookonly supports.xlsxfiles. The action description doesn't restrict to.xlsx, andCompareExcelFilesAndLogDifferencesin this PR usesWorkbookFactory.create()which handles both.xlsand.xlsx. Consider usingWorkbookFactoryfor consistency:♻️ Proposed fix
-import org.apache.poi.xssf.usermodel.XSSFWorkbook; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory;try (FileInputStream fis = new FileInputStream(excelFile); - XSSFWorkbook workbook = new XSSFWorkbook(fis)) { + Workbook workbook = WorkbookFactory.create(fis)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java` around lines 48 - 49, The code opens the Excel file using XSSFWorkbook which only supports .xlsx; update the try-with-resources in VerifySheetNameAtIndex to use WorkbookFactory.create(fis) (assign to a Workbook) so both .xls and .xlsx are supported, ensure the Workbook returned is closed by the same try-with-resources block, and remove the XSSFWorkbook-specific import if present; keep the FileInputStream in the same try block and adjust any type references from XSSFWorkbook to Workbook where used.
100-124: DuplicategetExcelFile/downloadFilemethods across all new action classes.These helper methods are identical in
VerifySheetNameAtIndex,VerifySheetNameExists, andCompareExcelFilesAndLogDifferences. SinceExcelCellUtilsalready providesdownloadUrlToTempFile, consider adding agetExcelFile(String path, Logger logger)utility there as well to eliminate duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java` around lines 100 - 124, These classes duplicate getExcelFile(String) and downloadFile(String); move this logic into a shared utility by adding ExcelCellUtils.getExcelFile(String path, Logger logger) that handles local vs URL paths (using existing ExcelCellUtils.downloadUrlToTempFile for downloads), returns a File or throws IOException, and logs via the provided logger; then remove the getExcelFile and downloadFile methods from VerifySheetNameAtIndex, VerifySheetNameExists, and CompareExcelFilesAndLogDifferences and replace their calls with ExcelCellUtils.getExcelFile(filePath, logger).excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.java (1)
45-46: UseWorkbookFactoryinstead ofXSSFWorkbookfor broader format support.Same concern as
VerifySheetNameAtIndex:XSSFWorkbookonly supports.xlsx. UseWorkbookFactory.create()for consistency withCompareExcelFilesAndLogDifferencesand to support both.xlsand.xlsxfiles.♻️ Proposed fix
-import org.apache.poi.xssf.usermodel.XSSFWorkbook; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory;try (FileInputStream fis = new FileInputStream(excelFile); - XSSFWorkbook workbook = new XSSFWorkbook(fis)) { + Workbook workbook = WorkbookFactory.create(fis)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.java` around lines 45 - 46, The code in VerifySheetNameExists currently constructs an XSSFWorkbook (which only supports .xlsx); replace that with WorkbookFactory.create(fis) to support both .xls and .xlsx like CompareExcelFilesAndLogDifferences does—open the FileInputStream, call Workbook wb = WorkbookFactory.create(fis) inside the try-with-resources (so the Workbook is closed automatically), update any XSSFWorkbook references to Workbook where needed, and import org.apache.poi.ss.usermodel.Workbook and org.apache.poi.ss.usermodel.WorkbookFactory; mirror the same pattern used in VerifySheetNameAtIndex/CompareExcelFilesAndLogDifferences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.java`:
- Around line 35-36: The extraction of rawName uses
Paths.get(url.getPath()).getFileName().toString() which can NPE when
getFileName() returns null for root-only paths; update the code in
ExcelCellUtils (where rawName and fileName are declared) to first store
Path#getFileName() into a variable, check it for null, and only call toString()
when non-null, otherwise assign a safe fallback (e.g., empty string or a default
name) to rawName before computing fileName.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/CompareExcelFilesAndLogDifferences.java`:
- Around line 51-52: The code calls getExcelFile(path1) and getExcelFile(path2)
which may create temp files for URL inputs but never deletes them; update the
workflow in CompareExcelFilesAndLogDifferences to track whether each returned
File is a temp (e.g., by having getExcelFile return a small holder or by
checking the input was a URL) and ensure both files are deleted in a finally
block after processing; wrap the main processing in try/finally, attempt safe
delete() on excelFile1 and excelFile2 if they were created as temps, and apply
the same cleanup change to the other usage at the later block (around lines
163-167).
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java`:
- Line 46: The temp file created by getExcelFile is never deleted; update the
usages (e.g., where File excelFile = getExcelFile(excelFilePath) in
VerifySheetNameAtIndex and the similar call later) to ensure the downloaded temp
file is cleaned up: after you finish using the File (parsing/reading the
workbook) delete it in a finally block or call file.deleteOnExit() immediately
after creation, and prefer explicit file.delete() in finally to free disk space;
ensure any streams opened when processing the file (e.g., workbook/input
streams) are closed before deleting so deletion succeeds.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.java`:
- Line 43: The temp file downloaded in VerifySheetNameExists (via
getExcelFile(excelFilePath)) is never deleted, causing a resource leak; update
the logic to ensure the temporary file is removed after use by adding cleanup in
a finally block (or try-with-resources) around the code that calls getExcelFile
in VerifySheetNameExists and around the similar block at lines 109-113, closing
streams and deleting the temp File (or calling file.delete() /
Files.deleteIfExists) even on exceptions; alternatively modify getExcelFile to
mark the File for deletion (deleteOnExit) and/or return a Closeable wrapper so
callers (VerifySheetNameExists and the code around 109-113) can reliably delete
or close the resource.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java`:
- Around line 41-45: The runtime-variable fields and assignments were removed
causing the action to no longer set the updated file path in the runtime
variable; restore the annotated fields (uncomment the `@TestData`(reference =
"variable-name", isRuntimeVariable = true) private com.testsigma.sdk.TestData
variableName; and the `@RunTimeData` private com.testsigma.sdk.RunTimeData
runTimeData;) and reintroduce the code that assigns the computed file path into
the runtime output (use variableName or runTimeData assignment where the code
now prints to stdout), removing the temporary System.out.println usage and
ensuring the same assignment occurs for the code paths referenced around the
earlier commented block and the similar spot at the region corresponding to
lines 115-117.
- Around line 47-55: Remove the embedded main method from the
WriteCellValueInNewSheetFilePath class that instantiates
WriteCellValueInNewSheetFilePath, calls setExcelPath with the hardcoded
presigned URL and other setTestData* methods, and then calls execute; delete
this debug entrypoint and all hardcoded sensitive tokens/URLs, and if an example
run is needed move it to a dedicated test or example fixture that reads the
Excel path from a secure configuration or test resource (use setExcelPath only
in tests and never commit presigned URLs).
---
Nitpick comments:
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java`:
- Around line 48-49: The code opens the Excel file using XSSFWorkbook which only
supports .xlsx; update the try-with-resources in VerifySheetNameAtIndex to use
WorkbookFactory.create(fis) (assign to a Workbook) so both .xls and .xlsx are
supported, ensure the Workbook returned is closed by the same try-with-resources
block, and remove the XSSFWorkbook-specific import if present; keep the
FileInputStream in the same try block and adjust any type references from
XSSFWorkbook to Workbook where used.
- Around line 100-124: These classes duplicate getExcelFile(String) and
downloadFile(String); move this logic into a shared utility by adding
ExcelCellUtils.getExcelFile(String path, Logger logger) that handles local vs
URL paths (using existing ExcelCellUtils.downloadUrlToTempFile for downloads),
returns a File or throws IOException, and logs via the provided logger; then
remove the getExcelFile and downloadFile methods from VerifySheetNameAtIndex,
VerifySheetNameExists, and CompareExcelFilesAndLogDifferences and replace their
calls with ExcelCellUtils.getExcelFile(filePath, logger).
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.java`:
- Around line 45-46: The code in VerifySheetNameExists currently constructs an
XSSFWorkbook (which only supports .xlsx); replace that with
WorkbookFactory.create(fis) to support both .xls and .xlsx like
CompareExcelFilesAndLogDifferences does—open the FileInputStream, call Workbook
wb = WorkbookFactory.create(fis) inside the try-with-resources (so the Workbook
is closed automatically), update any XSSFWorkbook references to Workbook where
needed, and import org.apache.poi.ss.usermodel.Workbook and
org.apache.poi.ss.usermodel.WorkbookFactory; mirror the same pattern used in
VerifySheetNameAtIndex/CompareExcelFilesAndLogDifferences.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3695b8d-8a63-4b8f-b976-7fff2e329afd
📒 Files selected for processing (11)
excelActions_cloud/pom.xmlexcelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/CompareExcelFilesAndLogDifferences.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/StoreNonEmptyCellCount.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/StoreNonEmptyCellCountForColumnOrRow.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/StoreNonEmptyCellCountWithSheet.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/VerifyIfCellValuesInExcel.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java
Comment on lines
+35
to
+36
| String rawName = Paths.get(url.getPath()).getFileName().toString(); | ||
| String fileName; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify filename extraction lacks null-guard before toString()
rg -n -C3 'Paths\.get\(url\.getPath\(\)\)\.getFileName\(\)\.toString\(\)' \
excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.javaRepository: testsigmahq/testsigma-addons
Length of output: 1795
🌐 Web query:
Java Path.getFileName() null return behavior trailing slash
💡 Result:
In Java's java.nio.file.Path, the getFileName method returns the name of the file or directory denoted by the path, which is the farthest name element from the root. It returns null if the path has zero name elements, such as for root paths like Path.of("/") or Path.of("C:/") on Windows. A trailing slash does not cause getFileName to return null. Path.of (or Paths.get) parses the string into name elements by splitting on the platform-specific separator (/ on Unix, \ on Windows), ignoring redundant trailing separators. For example: - Path.of("/var/log/").getFileName returns a Path representing "log" - Path.of("C:\dir\").getFileName returns a Path representing "dir" - Path.of("/var/log").getFileName also returns "log" This behavior is consistent across platforms and Java versions, as Path normalizes paths by treating trailing separators as not adding extra empty name elements. Root paths like "/" have zero name elements (only root), hence null; paths with directories have name elements regardless of trailing slash. Official documentation confirms: "Returns: a path representing the name of the file or directory, or null if this path has zero elements" (Java SE 26 docs). Examples from sources: - StackOverflow demo shows Path.of("...TestFolder\") returns "TestFolder" on Windows. - Tutorials like TheLinuxCode show Path.of("/var/log/") prints the filename "log".
Citations:
- 1: https://thelinuxcode.com/path-getfilename-method-in-java-practical-examples-edge-cases-and-real-world-patterns/
- 2: https://stackoverflow.com/questions/58970092/is-there-a-better-to-get-last-element-of-an-absolute-path-in-java
- 3: https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/nio/file/Path.html
- 4: https://www.baeldung.com/java-filename-full-path
Guard filename extraction for root-path URLs.
Line 35 can throw a NullPointerException when the URL path is root-only (for example, http://example.com/ where the path is /). The Path.getFileName() method returns null for paths with zero name elements, causing .toString() to fail. Add a null-guard before calling .toString().
💡 Proposed fix
- String rawName = Paths.get(url.getPath()).getFileName().toString();
+ Path path = Paths.get(url.getPath());
+ Path fileNamePath = path.getFileName();
+ String rawName = (fileNamePath == null || fileNamePath.toString().isBlank())
+ ? "excel.tmp"
+ : fileNamePath.toString();📝 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.
Suggested change
| String rawName = Paths.get(url.getPath()).getFileName().toString(); | |
| String fileName; | |
| Path path = Paths.get(url.getPath()); | |
| Path fileNamePath = path.getFileName(); | |
| String rawName = (fileNamePath == null || fileNamePath.toString().isBlank()) | |
| ? "excel.tmp" | |
| : fileNamePath.toString(); | |
| String fileName; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelCellUtils.java`
around lines 35 - 36, The extraction of rawName uses
Paths.get(url.getPath()).getFileName().toString() which can NPE when
getFileName() returns null for root-only paths; update the code in
ExcelCellUtils (where rawName and fileName are declared) to first store
Path#getFileName() into a variable, check it for null, and only call toString()
when non-null, otherwise assign a safe fallback (e.g., empty string or a default
name) to rawName before computing fileName.
Comment on lines
+51
to
+52
| File excelFile1 = getExcelFile(path1); | ||
| File excelFile2 = getExcelFile(path2); |
There was a problem hiding this comment.
Temp files downloaded from URLs are never cleaned up.
Both excelFile1 and excelFile2 could be temp files if URLs are provided. These files persist after execution. This is particularly concerning here since two temp files may be created per execution.
🛠️ Proposed fix
Track which files are temp files and clean them up in a finally block:
public com.testsigma.sdk.Result execute() {
logger.info("Initiating Excel file comparison");
com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS;
+ File excelFile1 = null;
+ File excelFile2 = null;
+ boolean isTempFile1 = false;
+ boolean isTempFile2 = false;
try {
String path1 = filePath1.getValue().toString();
String path2 = filePath2.getValue().toString();
logger.info("File 1: " + path1);
logger.info("File 2: " + path2);
- File excelFile1 = getExcelFile(path1);
- File excelFile2 = getExcelFile(path2);
+ isTempFile1 = path1.startsWith("http://") || path1.startsWith("https://");
+ isTempFile2 = path2.startsWith("http://") || path2.startsWith("https://");
+ excelFile1 = getExcelFile(path1);
+ excelFile2 = getExcelFile(path2);
// ... rest of try block
} catch (Exception e) {
// ...
+ } finally {
+ cleanupTempFile(excelFile1, isTempFile1);
+ cleanupTempFile(excelFile2, isTempFile2);
}
return result;
}
+ private void cleanupTempFile(File file, boolean isTempFile) {
+ if (isTempFile && file != null && file.exists()) {
+ if (!file.delete()) {
+ logger.warn("Failed to delete temp file: " + file.getAbsolutePath());
+ }
+ }
+ }Also applies to: 163-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/CompareExcelFilesAndLogDifferences.java`
around lines 51 - 52, The code calls getExcelFile(path1) and getExcelFile(path2)
which may create temp files for URL inputs but never deletes them; update the
workflow in CompareExcelFilesAndLogDifferences to track whether each returned
File is a temp (e.g., by having getExcelFile return a small holder or by
checking the input was a URL) and ensure both files are deleted in a finally
block after processing; wrap the main processing in try/finally, attempt safe
delete() on excelFile1 and excelFile2 if they were created as temps, and apply
the same cleanup change to the other usage at the later block (around lines
163-167).
| logger.info("Expected sheet index: " + expectedIndex); | ||
|
|
||
| // Load the Excel file | ||
| File excelFile = getExcelFile(excelFilePath); |
There was a problem hiding this comment.
Temp file downloaded from URL is never cleaned up.
When getExcelFile downloads from a URL, the temp file persists after the action completes. Over time, this can lead to disk space exhaustion, especially in automated test environments running many iterations.
Consider deleting the temp file in a finally block or using deleteOnExit():
🛠️ Proposed fix
public com.testsigma.sdk.Result execute() {
logger.info("Initiating sheet name at index verification");
com.testsigma.sdk.Result result = com.testsigma.sdk.Result.SUCCESS;
+ File excelFile = null;
+ boolean isTempFile = false;
try {
String excelFilePath = filePath.getValue().toString();
String expectedSheetName = sheetName.getValue().toString();
int expectedIndex = Integer.parseInt(sheetIndex.getValue().toString());
logger.info("Excel file path: " + excelFilePath);
logger.info("Expected sheet name: " + expectedSheetName);
logger.info("Expected sheet index: " + expectedIndex);
// Load the Excel file
- File excelFile = getExcelFile(excelFilePath);
+ excelFile = getExcelFile(excelFilePath);
+ isTempFile = excelFilePath.startsWith("http://") || excelFilePath.startsWith("https://");
// ... rest of try block
} catch (NumberFormatException e) {
// ...
} catch (Exception e) {
// ...
+ } finally {
+ if (isTempFile && excelFile != null && excelFile.exists()) {
+ if (!excelFile.delete()) {
+ logger.warn("Failed to delete temp file: " + excelFile.getAbsolutePath());
+ }
+ }
}
return result;
}Also applies to: 120-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameAtIndex.java`
at line 46, The temp file created by getExcelFile is never deleted; update the
usages (e.g., where File excelFile = getExcelFile(excelFilePath) in
VerifySheetNameAtIndex and the similar call later) to ensure the downloaded temp
file is cleaned up: after you finish using the File (parsing/reading the
workbook) delete it in a finally block or call file.deleteOnExit() immediately
after creation, and prefer explicit file.delete() in finally to free disk space;
ensure any streams opened when processing the file (e.g., workbook/input
streams) are closed before deleting so deletion succeeds.
| logger.info("Expected sheet name: " + expectedSheetName); | ||
|
|
||
| // Load the Excel file | ||
| File excelFile = getExcelFile(excelFilePath); |
There was a problem hiding this comment.
Temp file downloaded from URL is never cleaned up.
Same resource leak issue as VerifySheetNameAtIndex. The temp file created when downloading from a URL persists after execution. Consider adding cleanup logic in a finally block.
Also applies to: 109-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/VerifySheetNameExists.java`
at line 43, The temp file downloaded in VerifySheetNameExists (via
getExcelFile(excelFilePath)) is never deleted, causing a resource leak; update
the logic to ensure the temporary file is removed after use by adding cleanup in
a finally block (or try-with-resources) around the code that calls getExcelFile
in VerifySheetNameExists and around the similar block at lines 109-113, closing
streams and deleting the temp File (or calling file.delete() /
Files.deleteIfExists) even on exceptions; alternatively modify getExcelFile to
mark the File for deletion (deleteOnExit) and/or return a Closeable wrapper so
callers (VerifySheetNameExists and the code around 109-113) can reliably delete
or close the resource.
Comment on lines
+41
to
+45
| // @TestData(reference = "variable-name", isRuntimeVariable = true) | ||
| // private com.testsigma.sdk.TestData variableName; | ||
|
|
||
| // @RunTimeData | ||
| // private com.testsigma.sdk.RunTimeData runTimeData; |
There was a problem hiding this comment.
Restore runtime-variable output; current change breaks action behavior.
The action contract says the updated file path is stored in a runtime variable, but those fields and assignments are commented out and replaced with stdout logging.
💡 Proposed fix
- // `@TestData`(reference = "variable-name", isRuntimeVariable = true)
- // private com.testsigma.sdk.TestData variableName;
+ `@TestData`(reference = "variable-name", isRuntimeVariable = true)
+ private com.testsigma.sdk.TestData variableName;
- // `@RunTimeData`
- // private com.testsigma.sdk.RunTimeData runTimeData;
+ `@RunTimeData`
+ private com.testsigma.sdk.RunTimeData runTimeData;
@@
- System.out.println("Excel file path: " + excelFile.getAbsolutePath());
- // runTimeData.setKey(variableName.getValue().toString());
- // runTimeData.setValue(excelFile.getAbsolutePath());
+ runTimeData.setKey(variableName.getValue().toString());
+ runTimeData.setValue(excelFile.getAbsolutePath());Also applies to: 115-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java`
around lines 41 - 45, The runtime-variable fields and assignments were removed
causing the action to no longer set the updated file path in the runtime
variable; restore the annotated fields (uncomment the `@TestData`(reference =
"variable-name", isRuntimeVariable = true) private com.testsigma.sdk.TestData
variableName; and the `@RunTimeData` private com.testsigma.sdk.RunTimeData
runTimeData;) and reintroduce the code that assigns the computed file path into
the runtime output (use variableName or runTimeData assignment where the code
now prints to stdout), removing the temporary System.out.println usage and
ensuring the same assignment occurs for the code paths referenced around the
earlier commented block and the similar spot at the region corresponding to
lines 115-117.
Comment on lines
+47
to
+55
| public static void main(String[] args) { | ||
| WriteCellValueInNewSheetFilePath writeCellValueInNewSheetFilePath = new WriteCellValueInNewSheetFilePath(); | ||
| writeCellValueInNewSheetFilePath.setExcelPath(new com.testsigma.sdk.TestData("https://s3.amazonaws.com/permanent-attachments-production.testsigma.com/2817/uploads/1478/6264/File_Example_10_%281%29.xlsx?response-content-type=application%2Foctet-stream&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEKj%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEaCXVzLWVhc3QtMSJIMEYCIQDXakLi9G%2BqPl%2FUFinIeYva70rh2KcKPJu%2FC5D14FEXHgIhAKys21OtsBPd%2F2wpmJ3fCN44hq1iM3p3t7MKVsJOVgk0KsoFCHAQAxoMMTM0ODYzOTU2NzIxIgwqW%2FYSppuwRoqAEc4qpwWgkI5shnLsjGi2neumNwBM8JswYTMuuumSl9NLvMPbXeQ%2Fj0DrjMvUUfyvQNsIh%2FrEWY92jGejfxk0oyBDOBVGaE9ldXn79XF9CNGUqleTd9515regrKQu4nAk80gaQIuYznNhOQPWD1xc6n9S%2FLwclbBdetq6r0gLSiHiPbjQOjil8HoGrh%2BboZwBrfESd%2FmiFXyr36Wn9b2grHI3cyR%2F9qnW0ygbAGiM9qCttt9iuZ7Jb9dpIQnfSsyuMR8suqn4tdUjr2gqeksvwlDpX5T19wYyxIesImWaqf4d02JOI%2F4PukeOW%2Fo0hdz%2BJerlPDZQdPQSXCgrt%2BUOeYY%2F8BuPo7rYLgCUITDq7NDZz87RkKBV7x8i4hVDcWCDlvj8b39LdYqfOARaz1Lqz5t4Iw9Y6IhIk5CPq5pgl%2BL%2BeG3JjQ4mRYlGBf3ftdmWyzkb5oUvpSTC4Dxd5m7fAbxo327%2BWvXr%2B5VWPN92eV7l3ddCV8vGs8qC9VMkbygjbgRG6AwU5L0n0GTz9udFLf2nE%2F8GcYLMbJacR%2FvC%2BgS%2BQYfPc3LTiZpvyCRA5QwdpdOFrfvW9WvMOqu7Z9c2uvDEXEFC%2F9ViyuhqE0K9lLCobLWMw8wEsI7JmzHf7VW3yZs%2BInG%2FeWOtuVoBbDAOYPZlXam37Gcxptq2vt9b%2Fy7h2Cv6yZ4sKuLWcdf3u0TXBcJm70tLVSyx%2BjOZZ7skIcu0pQHsJxbvVY7pimgsZzts%2BKGDA6ocip0UbBlFiyM4rTmJbTWizRJMLlXmtScsgmD73nLiyS9JDhMKeNmqKZ3Nm%2BD%2Blernu%2FmP7YmxFitLojD3J8LmiUpxCm7c0fHmsrQOJ1vMrzVp4fKvPOtxsHrARDn50JXFHkZslWE78Xkn3p%2Bx9dxjhbgfbHSmMJeLus4GOo8B7f64maNE7S2YOvj4sTbo%2FAPXHpsrh6BYYILC004rSKJwRIl8gkxwITH2r%2FZQhdGeH1A1ZvqaCiiixjfRWVl5jhthdCLeT4KTqJXLH2wRdy9lfDRp%2BFkaJpYX29h%2F0RYNs29UiwkGyNGHArkR1cEOlJWAFNBv2p5L1EWK%2FA8TNDw92Bxzc2IuuN2nGvu%2FBnE%3D&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260402T181849Z&X-Amz-SignedHeaders=host&X-Amz-Credential=ASIAR6ZUEVLYWVLHGLDF%2F20260402%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Expires=10800&X-Amz-Signature=e9e02fcf2f7230c88f1b98ee751decc379e737620d2b875829b9f0eb540211fc")); | ||
| writeCellValueInNewSheetFilePath.setTestData1(new com.testsigma.sdk.TestData("1")); | ||
| writeCellValueInNewSheetFilePath.setTestData2(new com.testsigma.sdk.TestData("1")); | ||
| writeCellValueInNewSheetFilePath.setTestData3(new com.testsigma.sdk.TestData("aKHIL")); | ||
| writeCellValueInNewSheetFilePath.setTestData4(new com.testsigma.sdk.TestData("Sheet1")); | ||
| writeCellValueInNewSheetFilePath.execute(); | ||
| } |
There was a problem hiding this comment.
Remove the embedded main method with hardcoded presigned URL/token.
This commits sensitive tokenized URL material into source and introduces non-production debug execution code in the action class.
💡 Proposed fix
- public static void main(String[] args) {
- WriteCellValueInNewSheetFilePath writeCellValueInNewSheetFilePath = new WriteCellValueInNewSheetFilePath();
- writeCellValueInNewSheetFilePath.setExcelPath(new com.testsigma.sdk.TestData("https://s3.amazonaws.com/..."));
- writeCellValueInNewSheetFilePath.setTestData1(new com.testsigma.sdk.TestData("1"));
- writeCellValueInNewSheetFilePath.setTestData2(new com.testsigma.sdk.TestData("1"));
- writeCellValueInNewSheetFilePath.setTestData3(new com.testsigma.sdk.TestData("aKHIL"));
- writeCellValueInNewSheetFilePath.setTestData4(new com.testsigma.sdk.TestData("Sheet1"));
- writeCellValueInNewSheetFilePath.execute();
- }📝 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.
Suggested change
| public static void main(String[] args) { | |
| WriteCellValueInNewSheetFilePath writeCellValueInNewSheetFilePath = new WriteCellValueInNewSheetFilePath(); | |
| writeCellValueInNewSheetFilePath.setExcelPath(new com.testsigma.sdk.TestData("https://s3.amazonaws.com/permanent-attachments-production.testsigma.com/2817/uploads/1478/6264/File_Example_10_%281%29.xlsx?response-content-type=application%2Foctet-stream&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEKj%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEaCXVzLWVhc3QtMSJIMEYCIQDXakLi9G%2BqPl%2FUFinIeYva70rh2KcKPJu%2FC5D14FEXHgIhAKys21OtsBPd%2F2wpmJ3fCN44hq1iM3p3t7MKVsJOVgk0KsoFCHAQAxoMMTM0ODYzOTU2NzIxIgwqW%2FYSppuwRoqAEc4qpwWgkI5shnLsjGi2neumNwBM8JswYTMuuumSl9NLvMPbXeQ%2Fj0DrjMvUUfyvQNsIh%2FrEWY92jGejfxk0oyBDOBVGaE9ldXn79XF9CNGUqleTd9515regrKQu4nAk80gaQIuYznNhOQPWD1xc6n9S%2FLwclbBdetq6r0gLSiHiPbjQOjil8HoGrh%2BboZwBrfESd%2FmiFXyr36Wn9b2grHI3cyR%2F9qnW0ygbAGiM9qCttt9iuZ7Jb9dpIQnfSsyuMR8suqn4tdUjr2gqeksvwlDpX5T19wYyxIesImWaqf4d02JOI%2F4PukeOW%2Fo0hdz%2BJerlPDZQdPQSXCgrt%2BUOeYY%2F8BuPo7rYLgCUITDq7NDZz87RkKBV7x8i4hVDcWCDlvj8b39LdYqfOARaz1Lqz5t4Iw9Y6IhIk5CPq5pgl%2BL%2BeG3JjQ4mRYlGBf3ftdmWyzkb5oUvpSTC4Dxd5m7fAbxo327%2BWvXr%2B5VWPN92eV7l3ddCV8vGs8qC9VMkbygjbgRG6AwU5L0n0GTz9udFLf2nE%2F8GcYLMbJacR%2FvC%2BgS%2BQYfPc3LTiZpvyCRA5QwdpdOFrfvW9WvMOqu7Z9c2uvDEXEFC%2F9ViyuhqE0K9lLCobLWMw8wEsI7JmzHf7VW3yZs%2BInG%2FeWOtuVoBbDAOYPZlXam37Gcxptq2vt9b%2Fy7h2Cv6yZ4sKuLWcdf3u0TXBcJm70tLVSyx%2BjOZZ7skIcu0pQHsJxbvVY7pimgsZzts%2BKGDA6ocip0UbBlFiyM4rTmJbTWizRJMLlXmtScsgmD73nLiyS9JDhMKeNmqKZ3Nm%2BD%2Blernu%2FmP7YmxFitLojD3J8LmiUpxCm7c0fHmsrQOJ1vMrzVp4fKvPOtxsHrARDn50JXFHkZslWE78Xkn3p%2Bx9dxjhbgfbHSmMJeLus4GOo8B7f64maNE7S2YOvj4sTbo%2FAPXHpsrh6BYYILC004rSKJwRIl8gkxwITH2r%2FZQhdGeH1A1ZvqaCiiixjfRWVl5jhthdCLeT4KTqJXLH2wRdy9lfDRp%2BFkaJpYX29h%2F0RYNs29UiwkGyNGHArkR1cEOlJWAFNBv2p5L1EWK%2FA8TNDw92Bxzc2IuuN2nGvu%2FBnE%3D&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260402T181849Z&X-Amz-SignedHeaders=host&X-Amz-Credential=ASIAR6ZUEVLYWVLHGLDF%2F20260402%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Expires=10800&X-Amz-Signature=e9e02fcf2f7230c88f1b98ee751decc379e737620d2b875829b9f0eb540211fc")); | |
| writeCellValueInNewSheetFilePath.setTestData1(new com.testsigma.sdk.TestData("1")); | |
| writeCellValueInNewSheetFilePath.setTestData2(new com.testsigma.sdk.TestData("1")); | |
| writeCellValueInNewSheetFilePath.setTestData3(new com.testsigma.sdk.TestData("aKHIL")); | |
| writeCellValueInNewSheetFilePath.setTestData4(new com.testsigma.sdk.TestData("Sheet1")); | |
| writeCellValueInNewSheetFilePath.execute(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java`
around lines 47 - 55, Remove the embedded main method from the
WriteCellValueInNewSheetFilePath class that instantiates
WriteCellValueInNewSheetFilePath, calls setExcelPath with the hardcoded
presigned URL and other setTestData* methods, and then calls execute; delete
this debug entrypoint and all hardcoded sensitive tokens/URLs, and if an example
run is needed move it to a dedicated test or example fixture that reads the
Excel path from a secure configuration or test resource (use setExcelPath only
in tests and never commit presigned URLs).
vignesh-testsigma
approved these changes
Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Publish this addon as PUBLIC
Addon Name: ExcelActions_cloud
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-11720
Shorten temp filenames for Excel downloads from URLs
Summary by CodeRabbit
Release Notes
New Features
Chores