feat/CUS-11219-Added Post Plan Hook to Send TestData Profile Data as Excel#355
Conversation
📝 WalkthroughWalkthroughA new TestSigma hook add-on project that retrieves Test Data Profile data via API, generates Excel files for each profile, and sends them as email attachments to specified recipients using configured TestData inputs and SDK utilities. Changes
Sequence DiagramsequenceDiagram
participant Hook as SendTDPToMailHook
participant API as API Service
participant File as File System
participant Excel as Excel Writer
participant Email as Email Service
Hook->>Hook: Read TestData inputs<br/>(emails, subject, body, TDP IDs, API key)
Hook->>API: For each TDP ID: makeGetRequest(endpoint, id, apiKey)
API-->>Hook: TDP JSON response<br/>(with testDataName)
Hook->>File: Create temp file with<br/>safe filename
Hook->>Excel: writeToExcelFile(file, response)
Excel->>Excel: Parse JSON → Create sheet<br/>with headers & rows
Excel->>File: Write workbook to temp file
File-->>Excel: File written
Hook->>Email: Attach Excel files<br/>with subject & body
Email-->>Hook: Email sent
Hook-->>Hook: Return Result.SUCCESS<br/>or Result.FAILED
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
send_testdataprofile_as_excel_to_email_hook/pom.xml (3)
42-46: Add test scope to TestNG dependency and consider upgrading.TestNG is missing
<scope>test</scope>, causing it to be bundled in the shaded jar unnecessarily. Also, version 6.14.3 (from 2018) is significantly outdated.Suggested fix
<dependency> <groupId>org.testng</groupId> <artifactId>testng</artifactId> - <version>6.14.3</version> + <version>7.9.0</version> + <scope>test</scope> </dependency>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@send_testdataprofile_as_excel_to_email_hook/pom.xml` around lines 42 - 46, The TestNG dependency block (artifactId "testng", version "6.14.3") lacks a test scope and uses an outdated version; update the dependency to include <scope>test</scope> so it isn’t packaged into the shaded jar and bump the TestNG version to a recent stable release (e.g., 7.x) to get bug fixes and Java compatibility improvements; locate the dependency element for testng in the pom and add the scope node and adjust the version accordingly.
89-101: Consider adding filters to exclude signature files from shaded jar.When creating a shaded/fat jar that includes signed dependencies, the bundled signature files can cause
SecurityException: Invalid signature file digestat runtime. Consider adding filters to exclude these files.Suggested configuration
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.2.4</version> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> + <configuration> + <filters> + <filter> + <artifact>*:*</artifact> + <excludes> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + </excludes> + </filter> + </filters> + </configuration> </execution> </executions> </plugin>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@send_testdataprofile_as_excel_to_email_hook/pom.xml` around lines 89 - 101, The maven-shade-plugin configuration currently produces a fat jar that can include dependency signature files causing SecurityException at runtime; update the maven-shade-plugin (artifactId maven-shade-plugin) configuration to add a <filters> section that excludes signature files (e.g., exclude META-INF/*.SF, META-INF/*.DSA, META-INF/*.RSA and META-INF/*.SF*) for all artifacts (artifact="*:*") so those signature entries are removed from the shaded jar during the package execution.
17-17: Avoid using pre-release/milestone versions in production.
5.8.0-M1is a milestone (pre-release) version of JUnit Jupiter. Consider using a stable release like5.10.2or5.11.0for better stability and security.Suggested fix
- <junit.jupiter.version>5.8.0-M1</junit.jupiter.version> + <junit.jupiter.version>5.10.2</junit.jupiter.version>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@send_testdataprofile_as_excel_to_email_hook/pom.xml` at line 17, The pom currently pins the JUnit Jupiter property <junit.jupiter.version> to a milestone release (5.8.0-M1); update that property value to a stable release (for example 5.10.2 or 5.11.0) to avoid pre-release dependencies, then run mvn -q -DskipTests=true validate (or your CI build) to ensure no other modules require the old version and adjust any version constraints if tests report incompatibilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/SendTDPToMailHook.java`:
- Around line 91-93: The hook currently treats email.send() returning false as
success; update the block after Boolean mailSent = email.send() to treat null or
Boolean.FALSE as failure: check if (mailSent == null || !mailSent) then log an
error via logger.error with details, call the hook failure API (replace the
current setSuccessMessage call with setErrorMessage or the project’s failure
method) to mark the hook failed and return/stop further success handling; only
call setSuccessMessage("...") when mailSent is Boolean.TRUE. Ensure you
reference email.send(), logger, and setSuccessMessage/setErrorMessage in the
change.
- Around line 78-100: The temp .xlsx files created (File file in the loop and
added to destination_files) rely only on deleteOnExit(), which accumulates files
in long-lived JVMs; after calling email.send() (Boolean mailSent) you should
explicitly delete each File in destination_files (and log failures) and clear
the list (as a fallback you may keep deleteOnExit()), or perform deletion in a
finally block that runs whether send succeeds or fails; locate the
try-with-resources where XSSFWorkbook is created, the collection
destination_files, the write.writeToExcelFile(...) call, and the email.send() /
logger.info("Mail sent status - " + mailSent) area to add the cleanup loop that
attempts file.delete() and logs the result.
- Around line 59-68: The input extraction of apikey, tdpIds, emailIds,
emailSubject, and emailBody currently occurs before the try block (variables
apiKeyValue, tdpIdsValue, recipients, emailSubjectValue, emailBodyValue), so
missing inputs throw before your failure handling; move the calls that
dereference hook inputs (e.g., apikey.getValue(), tdpIds.getValue(),
emailIds.getValue(), emailSubject.getValue(), emailBody.getValue()) into the
existing try block, validate each for null/empty (and split tdpIds/emailIds
after null-check), and on validation failure call setErrorMessage(...) and
return Result.FAILED from within the try so errors follow the intended failure
path.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/ApiMethods.java`:
- Around line 12-25: The makeGetRequest method currently relies only on
HttpClient's connectTimeout (HttpClient client) so client.send() can hang if the
server accepts the connection but never sends a response; update the HttpRequest
creation in makeGetRequest to set a per-request timeout via
HttpRequest.Builder.timeout(Duration.ofSeconds(...)) (e.g., 10-30s) so each
request has an overall deadline, and ensure any TimeoutException or
InterruptedException is propagated or handled as appropriate.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/FileMethods.java`:
- Around line 22-58: The catch blocks in the Excel generation logic (inside
FileMethods, around workbook.createSheet(...) and the FileOutputStream write)
currently only call error.printStackTrace(), swallowing exceptions; remove these
swallowings and let exceptions propagate by rethrowing the caught Exception (or
wrap as IOException) so the method's throws IOException declaration is honored
and failures surface to execute(); specifically update the outer try-catch that
surrounds workbook creation and the inner try-catch around new
FileOutputStream(...) to either remove the catch entirely or replace
error.printStackTrace() with a throw of the error (or new IOException(error)) so
callers of the method (e.g., execute()) can detect and handle the failure.
- Around line 25-48: The code is using the first object's JSONObject keys
(columnNames) to write headers but then iterates each row's own keySet
(rowValues) to write cells, which can misalign values; fix by building one
deterministic ordered header list (e.g., a LinkedHashSet/List named headers)
before writing the sheet by iterating all arr elements and collecting all keys
from each element's "data" JSONObject, then write headers using that headers
list (replace SetValueCellId/header creation logic), and for each row iterate
the same headers list and look up each header in rowValues (use
rowValues.has(header) or a safe get) to write the corresponding cell or an empty
value; keep existing identifiers like columnNames, rowValues, setNameRowId,
SetValueCellId and sheet/row creation but change iteration to use the single
headers collection so columns stay deterministic and new keys are included.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/resources/testsigma-sdk.properties`:
- Line 1: The committed real API key in testsigma-sdk.api.key must be removed
and replaced with a non-secret placeholder (e.g.,
testsigma-sdk.api.key=PLACEHOLDER_TESTSIGMA_API_KEY) in the properties file;
after replacing the value in the file referenced by testsigma-sdk.api.key,
revoke/rotate the exposed bearer token in the Testsigma dashboard immediately
and update any secrets/storage (env vars or CI secret store) to use the new key
instead of committing it to source control.
---
Nitpick comments:
In `@send_testdataprofile_as_excel_to_email_hook/pom.xml`:
- Around line 42-46: The TestNG dependency block (artifactId "testng", version
"6.14.3") lacks a test scope and uses an outdated version; update the dependency
to include <scope>test</scope> so it isn’t packaged into the shaded jar and bump
the TestNG version to a recent stable release (e.g., 7.x) to get bug fixes and
Java compatibility improvements; locate the dependency element for testng in the
pom and add the scope node and adjust the version accordingly.
- Around line 89-101: The maven-shade-plugin configuration currently produces a
fat jar that can include dependency signature files causing SecurityException at
runtime; update the maven-shade-plugin (artifactId maven-shade-plugin)
configuration to add a <filters> section that excludes signature files (e.g.,
exclude META-INF/*.SF, META-INF/*.DSA, META-INF/*.RSA and META-INF/*.SF*) for
all artifacts (artifact="*:*") so those signature entries are removed from the
shaded jar during the package execution.
- Line 17: The pom currently pins the JUnit Jupiter property
<junit.jupiter.version> to a milestone release (5.8.0-M1); update that property
value to a stable release (for example 5.10.2 or 5.11.0) to avoid pre-release
dependencies, then run mvn -q -DskipTests=true validate (or your CI build) to
ensure no other modules require the old version and adjust any version
constraints if tests report incompatibilities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9b25680-50be-42df-a17f-78e2f5d603bb
📒 Files selected for processing (5)
send_testdataprofile_as_excel_to_email_hook/pom.xmlsend_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/SendTDPToMailHook.javasend_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/ApiMethods.javasend_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/FileMethods.javasend_testdataprofile_as_excel_to_email_hook/src/main/resources/testsigma-sdk.properties
| String apiKeyValue = apikey.getValue().toString(); | ||
| String[] tdpIdsValue = tdpIds.getValue().toString().split(","); | ||
|
|
||
| String[] recipients = emailIds.getValue().toString().split(","); | ||
| String emailSubjectValue = emailSubject.getValue().toString(); | ||
| String emailBodyValue = emailBody.getValue().toString(); | ||
|
|
||
| List<File> destination_files = new ArrayList<>(); | ||
|
|
||
| try { |
There was a problem hiding this comment.
Move required input extraction inside the guarded block.
These dereferences happen before the try, so a missing hook input throws immediately and bypasses your Result.FAILED/setErrorMessage(...) path. Extract and validate them inside the try so the hook fails cleanly.
Suggested change
- String apiKeyValue = apikey.getValue().toString();
- String[] tdpIdsValue = tdpIds.getValue().toString().split(",");
-
- String[] recipients = emailIds.getValue().toString().split(",");
- String emailSubjectValue = emailSubject.getValue().toString();
- String emailBodyValue = emailBody.getValue().toString();
-
List<File> destination_files = new ArrayList<>();
try {
+ String apiKeyValue = java.util.Objects.requireNonNull(apikey, "Testsigma API Key is required").getValue().toString();
+ String[] tdpIdsValue = java.util.Objects.requireNonNull(tdpIds, "Test Data Profile Ids are required").getValue().toString().split(",");
+ String[] recipients = java.util.Objects.requireNonNull(emailIds, "To Email Ids are required").getValue().toString().split(",");
+ String emailSubjectValue = java.util.Objects.requireNonNull(emailSubject, "Email Subject is required").getValue().toString();
+ String emailBodyValue = java.util.Objects.requireNonNull(emailBody, "Email Body is required").getValue().toString();
+
for (int i = 0; i < tdpIdsValue.length; i++) {📝 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.
| String apiKeyValue = apikey.getValue().toString(); | |
| String[] tdpIdsValue = tdpIds.getValue().toString().split(","); | |
| String[] recipients = emailIds.getValue().toString().split(","); | |
| String emailSubjectValue = emailSubject.getValue().toString(); | |
| String emailBodyValue = emailBody.getValue().toString(); | |
| List<File> destination_files = new ArrayList<>(); | |
| try { | |
| List<File> destination_files = new ArrayList<>(); | |
| try { | |
| String apiKeyValue = java.util.Objects.requireNonNull(apikey, "Testsigma API Key is required").getValue().toString(); | |
| String[] tdpIdsValue = java.util.Objects.requireNonNull(tdpIds, "Test Data Profile Ids are required").getValue().toString().split(","); | |
| String[] recipients = java.util.Objects.requireNonNull(emailIds, "To Email Ids are required").getValue().toString().split(","); | |
| String emailSubjectValue = java.util.Objects.requireNonNull(emailSubject, "Email Subject is required").getValue().toString(); | |
| String emailBodyValue = java.util.Objects.requireNonNull(emailBody, "Email Body is required").getValue().toString(); | |
| for (int i = 0; i < tdpIdsValue.length; i++) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/SendTDPToMailHook.java`
around lines 59 - 68, The input extraction of apikey, tdpIds, emailIds,
emailSubject, and emailBody currently occurs before the try block (variables
apiKeyValue, tdpIdsValue, recipients, emailSubjectValue, emailBodyValue), so
missing inputs throw before your failure handling; move the calls that
dereference hook inputs (e.g., apikey.getValue(), tdpIds.getValue(),
emailIds.getValue(), emailSubject.getValue(), emailBody.getValue()) into the
existing try block, validate each for null/empty (and split tdpIds/emailIds
after null-check), and on validation failure call setErrorMessage(...) and
return Result.FAILED from within the try so errors follow the intended failure
path.
| File file = new File(System.getProperty("java.io.tmpdir"), safeTdpName + "_" + tdpId + ".xlsx"); | ||
| file.deleteOnExit(); | ||
|
|
||
| try (XSSFWorkbook workbook = new XSSFWorkbook()) { | ||
| write.writeToExcelFile(file, testDataResponse, workbook); | ||
| } | ||
| destination_files.add(file); | ||
| } | ||
|
|
||
| email.setTo(Arrays.asList(recipients)); | ||
| email.setSubject(emailSubjectValue + " For the RunResult Id: " + runResult.getId()); | ||
| email.setBody(emailBodyValue); | ||
| email.setAttachments(destination_files); | ||
| Boolean mailSent = email.send(); | ||
| logger.info("Mail sent status - " + mailSent); | ||
| setSuccessMessage("The specified test data profiles have been extracted and sent to the respective emails."); | ||
|
|
||
| } catch (Exception e) { | ||
| logger.info("Error in sending email: " + ExceptionUtils.getStackTrace(e)); | ||
| setErrorMessage("Error in sending email: " + ExceptionUtils.getMessage(e)); | ||
| result = Result.FAILED; | ||
| } | ||
| return result; |
There was a problem hiding this comment.
Clean up temp attachments after each execution.
deleteOnExit() only deletes files when the JVM exits. In a long-lived runner, these .xlsx files accumulate across hook executions and can eventually fill the temp directory.
Suggested change
- File file = new File(System.getProperty("java.io.tmpdir"), safeTdpName + "_" + tdpId + ".xlsx");
- file.deleteOnExit();
+ File file = new File(System.getProperty("java.io.tmpdir"), safeTdpName + "_" + tdpId + ".xlsx");
@@
} catch (Exception e) {
logger.info("Error in sending email: " + ExceptionUtils.getStackTrace(e));
setErrorMessage("Error in sending email: " + ExceptionUtils.getMessage(e));
result = Result.FAILED;
+ } finally {
+ for (File file : destination_files) {
+ if (file.exists() && !file.delete()) {
+ logger.info("Failed to delete temp file: " + file.getAbsolutePath());
+ }
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/SendTDPToMailHook.java`
around lines 78 - 100, The temp .xlsx files created (File file in the loop and
added to destination_files) rely only on deleteOnExit(), which accumulates files
in long-lived JVMs; after calling email.send() (Boolean mailSent) you should
explicitly delete each File in destination_files (and log failures) and clear
the list (as a fallback you may keep deleteOnExit()), or perform deletion in a
finally block that runs whether send succeeds or fails; locate the
try-with-resources where XSSFWorkbook is created, the collection
destination_files, the write.writeToExcelFile(...) call, and the email.send() /
logger.info("Mail sent status - " + mailSent) area to add the cleanup loop that
attempts file.delete() and logs the result.
| Boolean mailSent = email.send(); | ||
| logger.info("Mail sent status - " + mailSent); | ||
| setSuccessMessage("The specified test data profiles have been extracted and sent to the respective emails."); |
There was a problem hiding this comment.
Treat email.send() returning false as a failed hook.
Right now the hook reports success even when the SDK declines to send and returns false, which leaves the plan green with no email delivered.
Suggested change
Boolean mailSent = email.send();
logger.info("Mail sent status - " + mailSent);
+ if (!Boolean.TRUE.equals(mailSent)) {
+ throw new IllegalStateException("Email send returned false");
+ }
setSuccessMessage("The specified test data profiles have been extracted and sent to the respective emails.");📝 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.
| Boolean mailSent = email.send(); | |
| logger.info("Mail sent status - " + mailSent); | |
| setSuccessMessage("The specified test data profiles have been extracted and sent to the respective emails."); | |
| Boolean mailSent = email.send(); | |
| logger.info("Mail sent status - " + mailSent); | |
| if (!Boolean.TRUE.equals(mailSent)) { | |
| throw new IllegalStateException("Email send returned false"); | |
| } | |
| setSuccessMessage("The specified test data profiles have been extracted and sent to the respective emails."); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/SendTDPToMailHook.java`
around lines 91 - 93, The hook currently treats email.send() returning false as
success; update the block after Boolean mailSent = email.send() to treat null or
Boolean.FALSE as failure: check if (mailSent == null || !mailSent) then log an
error via logger.error with details, call the hook failure API (replace the
current setSuccessMessage call with setErrorMessage or the project’s failure
method) to mark the hook failed and return/stop further success handling; only
call setSuccessMessage("...") when mailSent is Boolean.TRUE. Ensure you
reference email.send(), logger, and setSuccessMessage/setErrorMessage in the
change.
| HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build(); | ||
| public String makeGetRequest(String endPoint, String id, String apiKey) throws IOException, InterruptedException, ApiException { | ||
|
|
||
| HttpResponse<String> response =null; | ||
| String body = null; | ||
| URI uri = URI.create(endPoint + id); | ||
| HttpRequest request = HttpRequest.newBuilder() | ||
| .uri(uri) | ||
| .GET() | ||
| .header("accept", "application/json") | ||
| .header("Content-Type", "application/json") | ||
| .header("Authorization", "Bearer " + apiKey) | ||
| .build(); | ||
| response = client.send(request, HttpResponse.BodyHandlers.ofString()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -e java "ApiMethods.java" 2>/dev/null | head -20Repository: testsigmahq/testsigma-addons
Length of output: 1907
🌐 Web query:
Java HttpClient connectTimeout vs request timeout timeout() method behavior
💡 Result:
-
HttpClient.Builder.connectTimeout(Duration): limits only the time to establish a new connection (TCP + any required setup). If the client reuses an existing connection, this timeout does not apply. If it expires,send()/sendAsync()fails withHttpConnectTimeoutException. [1] -
HttpRequest.Builder.timeout(Duration)(“request timeout”): a per-request deadline. If “the response is not received” within the timeout,send()throwsHttpTimeoutException(orsendAsync()completes exceptionally). If you don’t set it, it can wait indefinitely. [2] -
Important nuance (JDK behavior change):
- Up to current releases (JDK 11–25), the request timeout historically applied only until response headers are received, so a server that sends headers quickly but streams the body slowly could run much longer than the timeout. This is tracked in JDK-8208693. [3][4]
- JDK 26 changes this so the
HttpRequest.Builder::timeoutno longer stops at headers (i.e., it covers the body too). [4]
Rule of thumb: use connectTimeout to cap “can’t establish connection”, and request.timeout to cap overall request waiting time—but be aware that what counts as “received” differs by JDK version (pre-26 vs 26+). [1][2][4]
Sources
- Oracle Javadoc:
HttpClient.Builder#connectTimeout(Java SE) — connection-establishment-only; no effect on reused connections;HttpConnectTimeoutException. [1] - Oracle Javadoc:
HttpRequest.Builder#timeout— throwsHttpTimeoutExceptionif response not received in time; otherwise infinite. [2] - OpenJDK bug JDK-8208693 — request timeout stops when response headers are received (prior behavior). [3]
- Inside.java (Mar 4, 2026): “HTTP Client Updates in Java 26” — documents the change to request-timeout semantics in JDK 26. [4]
Add a request timeout, not just a connect timeout.
connectTimeout only covers establishing the TCP connection. If the API accepts the connection and then stalls on sending the response, client.send() can block indefinitely without a per-request timeout. Use HttpRequest.Builder.timeout() to add an overall request deadline.
Suggested change
HttpRequest request = HttpRequest.newBuilder()
.uri(uri)
+ .timeout(Duration.ofSeconds(30))
.GET()
.header("accept", "application/json")
.header("Content-Type", "application/json")
.header("Authorization", "Bearer " + apiKey)
.build();📝 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.
| HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build(); | |
| public String makeGetRequest(String endPoint, String id, String apiKey) throws IOException, InterruptedException, ApiException { | |
| HttpResponse<String> response =null; | |
| String body = null; | |
| URI uri = URI.create(endPoint + id); | |
| HttpRequest request = HttpRequest.newBuilder() | |
| .uri(uri) | |
| .GET() | |
| .header("accept", "application/json") | |
| .header("Content-Type", "application/json") | |
| .header("Authorization", "Bearer " + apiKey) | |
| .build(); | |
| response = client.send(request, HttpResponse.BodyHandlers.ofString()); | |
| HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(10)).build(); | |
| public String makeGetRequest(String endPoint, String id, String apiKey) throws IOException, InterruptedException, ApiException { | |
| HttpResponse<String> response =null; | |
| String body = null; | |
| URI uri = URI.create(endPoint + id); | |
| HttpRequest request = HttpRequest.newBuilder() | |
| .uri(uri) | |
| .timeout(Duration.ofSeconds(30)) | |
| .GET() | |
| .header("accept", "application/json") | |
| .header("Content-Type", "application/json") | |
| .header("Authorization", "Bearer " + apiKey) | |
| .build(); | |
| response = client.send(request, HttpResponse.BodyHandlers.ofString()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/ApiMethods.java`
around lines 12 - 25, The makeGetRequest method currently relies only on
HttpClient's connectTimeout (HttpClient client) so client.send() can hang if the
server accepts the connection but never sends a response; update the HttpRequest
creation in makeGetRequest to set a per-request timeout via
HttpRequest.Builder.timeout(Duration.ofSeconds(...)) (e.g., 10-30s) so each
request has an overall deadline, and ensure any TimeoutException or
InterruptedException is propagated or handled as appropriate.
| try { | ||
| XSSFSheet sheet = workbook.createSheet(testDataName); | ||
| JSONArray arr = obj.getJSONArray("data"); | ||
| JSONObject columnNames = (JSONObject) arr.getJSONObject(0).get("data"); | ||
| // Adding all the keys as column | ||
| int setNameRowId = 0; | ||
| int SetValueCellId = 0; | ||
| XSSFRow row = sheet.createRow(setNameRowId++); | ||
| Cell cell = row.createCell(SetValueCellId++); | ||
| cell.setCellValue("Set Name"); | ||
| for (String key : columnNames.keySet()) { | ||
| Cell headerCell = row.createCell(SetValueCellId++); | ||
| headerCell.setCellValue(key); | ||
| } | ||
|
|
||
| // Adding the data set name and its corresponding values | ||
| for (int i=0; i <arr.length(); i++) { | ||
| int dataSetRow = setNameRowId + i; | ||
| int dataSetCell = 0; | ||
| XSSFRow setRow = sheet.createRow(dataSetRow++); | ||
| String setValues = arr.getJSONObject(i).get("name").toString(); | ||
| Cell setValueCell = setRow.createCell(dataSetCell++); | ||
| setValueCell.setCellValue(setValues); | ||
| JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data"); | ||
| for (String key : rowValues.keySet()) { | ||
| Cell newCell = setRow.createCell(dataSetCell++); | ||
| newCell.setCellValue(rowValues.get(key).toString()); | ||
| } | ||
| } | ||
| try (FileOutputStream out = new FileOutputStream(new File(String.valueOf(file)))) { | ||
| workbook.write(out); | ||
| out.close(); | ||
| } catch (Exception error) { | ||
| error.printStackTrace(); | ||
| } | ||
| } catch (Exception error){ | ||
| error.printStackTrace(); |
There was a problem hiding this comment.
Propagate Excel-generation failures instead of swallowing them.
Both catch blocks only print the stack trace, so the hook can continue, attach a half-written file, and still return success. This method already declares throws IOException; let failures bubble up so execute() can mark the hook as failed.
Suggested change
- try {
+ try {
XSSFSheet sheet = workbook.createSheet(testDataName);
JSONArray arr = obj.getJSONArray("data");
@@
- try (FileOutputStream out = new FileOutputStream(new File(String.valueOf(file)))) {
+ try (FileOutputStream out = new FileOutputStream(file)) {
workbook.write(out);
- out.close();
- } catch (Exception error) {
- error.printStackTrace();
}
- } catch (Exception error){
- error.printStackTrace();
+ } catch (Exception error) {
+ throw new IOException("Failed to generate Excel for test data profile: " + testDataName, error);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/FileMethods.java`
around lines 22 - 58, The catch blocks in the Excel generation logic (inside
FileMethods, around workbook.createSheet(...) and the FileOutputStream write)
currently only call error.printStackTrace(), swallowing exceptions; remove these
swallowings and let exceptions propagate by rethrowing the caught Exception (or
wrap as IOException) so the method's throws IOException declaration is honored
and failures surface to execute(); specifically update the outer try-catch that
surrounds workbook creation and the inner try-catch around new
FileOutputStream(...) to either remove the catch entirely or replace
error.printStackTrace() with a throw of the error (or new IOException(error)) so
callers of the method (e.g., execute()) can detect and handle the failure.
| JSONObject columnNames = (JSONObject) arr.getJSONObject(0).get("data"); | ||
| // Adding all the keys as column | ||
| int setNameRowId = 0; | ||
| int SetValueCellId = 0; | ||
| XSSFRow row = sheet.createRow(setNameRowId++); | ||
| Cell cell = row.createCell(SetValueCellId++); | ||
| cell.setCellValue("Set Name"); | ||
| for (String key : columnNames.keySet()) { | ||
| Cell headerCell = row.createCell(SetValueCellId++); | ||
| headerCell.setCellValue(key); | ||
| } | ||
|
|
||
| // Adding the data set name and its corresponding values | ||
| for (int i=0; i <arr.length(); i++) { | ||
| int dataSetRow = setNameRowId + i; | ||
| int dataSetCell = 0; | ||
| XSSFRow setRow = sheet.createRow(dataSetRow++); | ||
| String setValues = arr.getJSONObject(i).get("name").toString(); | ||
| Cell setValueCell = setRow.createCell(dataSetCell++); | ||
| setValueCell.setCellValue(setValues); | ||
| JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data"); | ||
| for (String key : rowValues.keySet()) { | ||
| Cell newCell = setRow.createCell(dataSetCell++); | ||
| newCell.setCellValue(rowValues.get(key).toString()); |
There was a problem hiding this comment.
Build one deterministic header list and reuse it for every row.
The current code writes headers from the first object's keys, then writes each row using that row's own key iteration. If the per-row key order differs, values land under the wrong headers; if later rows introduce new keys, those columns are lost entirely.
Suggested change
- JSONObject columnNames = (JSONObject) arr.getJSONObject(0).get("data");
+ java.util.LinkedHashSet<String> headers = new java.util.LinkedHashSet<>();
+ for (int i = 0; i < arr.length(); i++) {
+ headers.addAll(arr.getJSONObject(i).getJSONObject("data").keySet());
+ }
// Adding all the keys as column
int setNameRowId = 0;
int SetValueCellId = 0;
XSSFRow row = sheet.createRow(setNameRowId++);
Cell cell = row.createCell(SetValueCellId++);
cell.setCellValue("Set Name");
- for (String key : columnNames.keySet()) {
+ for (String key : headers) {
Cell headerCell = row.createCell(SetValueCellId++);
headerCell.setCellValue(key);
}
@@
- JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data");
- for (String key : rowValues.keySet()) {
+ JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data");
+ for (String key : headers) {
Cell newCell = setRow.createCell(dataSetCell++);
- newCell.setCellValue(rowValues.get(key).toString());
+ newCell.setCellValue(rowValues.optString(key, ""));
}📝 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.
| JSONObject columnNames = (JSONObject) arr.getJSONObject(0).get("data"); | |
| // Adding all the keys as column | |
| int setNameRowId = 0; | |
| int SetValueCellId = 0; | |
| XSSFRow row = sheet.createRow(setNameRowId++); | |
| Cell cell = row.createCell(SetValueCellId++); | |
| cell.setCellValue("Set Name"); | |
| for (String key : columnNames.keySet()) { | |
| Cell headerCell = row.createCell(SetValueCellId++); | |
| headerCell.setCellValue(key); | |
| } | |
| // Adding the data set name and its corresponding values | |
| for (int i=0; i <arr.length(); i++) { | |
| int dataSetRow = setNameRowId + i; | |
| int dataSetCell = 0; | |
| XSSFRow setRow = sheet.createRow(dataSetRow++); | |
| String setValues = arr.getJSONObject(i).get("name").toString(); | |
| Cell setValueCell = setRow.createCell(dataSetCell++); | |
| setValueCell.setCellValue(setValues); | |
| JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data"); | |
| for (String key : rowValues.keySet()) { | |
| Cell newCell = setRow.createCell(dataSetCell++); | |
| newCell.setCellValue(rowValues.get(key).toString()); | |
| java.util.LinkedHashSet<String> headers = new java.util.LinkedHashSet<>(); | |
| for (int i = 0; i < arr.length(); i++) { | |
| headers.addAll(arr.getJSONObject(i).getJSONObject("data").keySet()); | |
| } | |
| // Adding all the keys as column | |
| int setNameRowId = 0; | |
| int SetValueCellId = 0; | |
| XSSFRow row = sheet.createRow(setNameRowId++); | |
| Cell cell = row.createCell(SetValueCellId++); | |
| cell.setCellValue("Set Name"); | |
| for (String key : headers) { | |
| Cell headerCell = row.createCell(SetValueCellId++); | |
| headerCell.setCellValue(key); | |
| } | |
| // Adding the data set name and its corresponding values | |
| for (int i=0; i <arr.length(); i++) { | |
| int dataSetRow = setNameRowId + i; | |
| int dataSetCell = 0; | |
| XSSFRow setRow = sheet.createRow(dataSetRow++); | |
| String setValues = arr.getJSONObject(i).get("name").toString(); | |
| Cell setValueCell = setRow.createCell(dataSetCell++); | |
| setValueCell.setCellValue(setValues); | |
| JSONObject rowValues = (JSONObject) arr.getJSONObject(i).get("data"); | |
| for (String key : headers) { | |
| Cell newCell = setRow.createCell(dataSetCell++); | |
| newCell.setCellValue(rowValues.optString(key, "")); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/java/com/testsigma/addons/hook/utility/FileMethods.java`
around lines 25 - 48, The code is using the first object's JSONObject keys
(columnNames) to write headers but then iterates each row's own keySet
(rowValues) to write cells, which can misalign values; fix by building one
deterministic ordered header list (e.g., a LinkedHashSet/List named headers)
before writing the sheet by iterating all arr elements and collecting all keys
from each element's "data" JSONObject, then write headers using that headers
list (replace SetValueCellId/header creation logic), and for each row iterate
the same headers list and look up each header in rowValues (use
rowValues.has(header) or a safe get) to write the corresponding cell or an empty
value; keep existing identifiers like columnNames, rowValues, setNameRowId,
SetValueCellId and sheet/row creation but change iteration to use the single
headers collection so columns stay deterministic and new keys are included.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyMjMyMmM2Ni04NWYzLWIyN2UtN2FiOS0zM2U2M2Q4OWM1MGIiLCJ1bmlxdWVJZCI6IjYwNjUiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiMzUifQ.5mfSXCDjLRAmgrNGVBdrmRu2mRwTWEkX2XIUkWoS9mnKFPhprfb-6Jt3c0UasGBAo2E_KVKIwRQtJMCdxAyvkg No newline at end of file | |||
There was a problem hiding this comment.
Remove and rotate the committed API key before merge.
This is a real bearer token in source control. Because this add-on is being published publicly, anyone who can read the repo can reuse it against the Testsigma API. Please replace it with a non-secret placeholder and revoke/rotate the exposed key.
Suggested change
-testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyMjMyMmM2Ni04NWYzLWIyN2UtN2FiOS0zM2U2M2Q4OWM1MGIiLCJ1bmlxdWVJZCI6IjYwNjUiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiMzUifQ.5mfSXCDjLRAmgrNGVBdrmRu2mRwTWEkX2XIUkWoS9mnKFPhprfb-6Jt3c0UasGBAo2E_KVKIwRQtJMCdxAyvkg
+testsigma-sdk.api.key=<configure-at-runtime>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@send_testdataprofile_as_excel_to_email_hook/src/main/resources/testsigma-sdk.properties`
at line 1, The committed real API key in testsigma-sdk.api.key must be removed
and replaced with a non-secret placeholder (e.g.,
testsigma-sdk.api.key=PLACEHOLDER_TESTSIGMA_API_KEY) in the properties file;
after replacing the value in the file referenced by testsigma-sdk.api.key,
revoke/rotate the exposed bearer token in the Testsigma dashboard immediately
and update any secrets/storage (env vars or CI secret store) to use the new key
instead of committing it to source control.
Publish this addon as PUBLIC
Addon Name: Send TestDataProfile As Excel To Email Hook
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-11219
Added Post Plan Hook to Send TestData Profile Data as Excel
Summary by CodeRabbit