feat/CUS-9600-Get latest downloaded file path from chrome and edge downloads#280
Conversation
WalkthroughThis PR introduces a new Maven module for a Testsigma WebAction that retrieves the latest downloaded file path from a browser's downloads. It implements browser-specific utilities for Chrome and Edge via a factory pattern, converts downloaded files to local copies, and stores the file path in a runtime variable. Changes
Sequence DiagramsequenceDiagram
participant WA as WebAction<br/>GetLatestDownloaded
participant UF as UtilitiesFactory
participant BU as Browser Utility<br/>(Chrome/Edge)
participant DL as Downloads UI
participant FS as File System
WA->>UF: create(driver, logger)
UF->>UF: detectBrowser()
alt Chrome detected
UF->>BU: new ChromeUtilities()
else Edge detected
UF->>BU: new EdgeUtilities()
end
UF-->>WA: Utilities instance
WA->>BU: copyFileFromDownloads()
BU->>DL: openTab(browser://downloads)
loop Wait for download
BU->>DL: isFileDownloaded()
DL->>DL: executeScript(shadowDOM)
DL-->>BU: false/true
end
BU->>DL: getDownloadedFileLocalPath()
DL->>DL: executeScript(extract path)
DL-->>BU: file path
BU->>FS: createLocalFileFromDownloadsCopy()
FS->>FS: base64 decode, write temp file
FS-->>BU: File object
BU->>BU: cleanup tabs/windows
BU-->>WA: File
WA->>WA: store path in RunTimeData
WA-->>WA: return SUCCESS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
get_latest_downloaded_file_path/pom.xml (2)
17-17: Avoid using milestone/pre-release versions in production.JUnit
5.8.0-M1is a milestone release. Consider using a stable version like5.10.0or later for production code.-<junit.jupiter.version>5.8.0-M1</junit.jupiter.version> +<junit.jupiter.version>5.10.0</junit.jupiter.version>
42-46: Add test scope to TestNG dependency.TestNG is missing the
<scope>test</scope>declaration, so it will be included in the production JAR. Also, both JUnit Jupiter and TestNG are declared as test frameworks, which is unusual.<dependency> <groupId>org.testng</groupId> <artifactId>testng</artifactId> <version>6.14.3</version> + <scope>test</scope> </dependency>get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/UtilitiesFactory.java (1)
9-18: Add null check and handle potential ClassCastException.The direct cast to
RemoteWebDriverwill fail with aClassCastExceptionif the driver is not aRemoteWebDriverinstance. While this may be fine for the expected use case, a more defensive approach would improve error messages.public static Utilities create(WebDriver driver, Logger logger) { + if (!(driver instanceof RemoteWebDriver)) { + throw new RuntimeException("Driver must be a RemoteWebDriver instance"); + } String browserName = ((RemoteWebDriver) driver).getCapabilities().getBrowserName().toLowerCase();get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFilePath.java (1)
42-42: Use appropriate log level for exceptions.Stack traces logged at INFO level may be filtered out in production. Consider using
logger.warn()orlogger.error()for exception logging.-logger.info("Exception Occurred: " + ExceptionUtils.getStackTrace(e)); +logger.warn("Exception Occurred: " + ExceptionUtils.getStackTrace(e));get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/EdgeUtilities.java (2)
40-41: Redundant exception wrapping.Catching
RuntimeExceptiononly to re-wrap it in anotherRuntimeExceptionadds no value and duplicates the stack trace.-} catch (RuntimeException e) { - throw new RuntimeException(e.getMessage(), e); -} finally { +} finally {The
RuntimeExceptionwill propagate naturally without explicit catch/rethrow.
102-102: Use appropriate log level."Switched to downloads page" is an informational message, not a warning.
-logger.warn("Switched to downloads page."); +logger.info("Switched to downloads page.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
get_latest_downloaded_file_path/pom.xml(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFilePath.java(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/BaseUtilities.java(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/ChromeUtilities.java(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/EdgeUtilities.java(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/Utilities.java(1 hunks)get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/UtilitiesFactory.java(1 hunks)get_latest_downloaded_file_path/src/main/resources/testsigma-sdk.properties(1 hunks)
🔇 Additional comments (2)
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/Utilities.java (1)
5-12: LGTM!Clean interface design with a clear separation of concerns. The three methods provide a well-defined contract for browser-specific implementations.
get_latest_downloaded_file_path/pom.xml (1)
48-52: Selenium version 4.33.0 is available on Maven Central and is a legitimate release. No changes needed.
| @Override | ||
| protected Result execute() throws NoSuchElementException { | ||
| Result result = Result.SUCCESS; | ||
| Utilities utilities = UtilitiesFactory.create(driver, logger); |
There was a problem hiding this comment.
Move factory creation inside try block.
UtilitiesFactory.create() can throw RuntimeException (for unsupported browsers or ClassCastException), but it's called outside the try block, so these exceptions won't be caught and will propagate unhandled.
protected Result execute() throws NoSuchElementException {
Result result = Result.SUCCESS;
- Utilities utilities = UtilitiesFactory.create(driver, logger);
try {
logger.info("Initiated execution");
+ Utilities utilities = UtilitiesFactory.create(driver, logger);
File filePath = utilities.copyFileFromDownloads();📝 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.
| Utilities utilities = UtilitiesFactory.create(driver, logger); | |
| protected Result execute() throws NoSuchElementException { | |
| Result result = Result.SUCCESS; | |
| try { | |
| logger.info("Initiated execution"); | |
| Utilities utilities = UtilitiesFactory.create(driver, logger); | |
| File filePath = utilities.copyFileFromDownloads(); |
🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/GetLatestDownloadedFilePath.java
around line 31, the call to UtilitiesFactory.create(driver, logger) can throw a
RuntimeException (e.g., unsupported browser or ClassCastException) but is
currently outside the try block; move the UtilitiesFactory.create(...) call
inside the existing try block and surround it so any RuntimeException is caught
by the catch clause (or add a specific catch for RuntimeException), then log the
exception, set the step/result to failure (or return early) and avoid letting
the RuntimeException propagate uncaught.
| assert elem != null; | ||
| elem.sendKeys(path); |
There was a problem hiding this comment.
Replace assert with explicit null check.
Same issue as in ChromeUtilities - assertions are disabled by default in production.
-assert elem != null;
-elem.sendKeys(path);
+if (elem == null) {
+ throw new RuntimeException("Failed to create file input element");
+}
+elem.sendKeys(path);📝 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.
| assert elem != null; | |
| elem.sendKeys(path); | |
| if (elem == null) { | |
| throw new RuntimeException("Failed to create file input element"); | |
| } | |
| elem.sendKeys(path); |
🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/BaseUtilities.java
around lines 33-34, replace the assertion with an explicit null check: verify
elem is not null and if it is, throw a clear runtime exception (for example
IllegalStateException or NullPointerException with a descriptive message) before
calling elem.sendKeys(path); alternatively use Objects.requireNonNull(elem,
"element was null while attempting to send keys") to fail-fast with a clear
message.
| String fileFormat = fileName.substring(fileName.lastIndexOf(".") + 1); | ||
|
|
||
| File downloadedFile = File.createTempFile(fileName.replace("." + fileFormat, ""), "." + fileFormat); |
There was a problem hiding this comment.
Handle files without extensions.
If the filename has no extension (e.g., Makefile), lastIndexOf(".") returns -1, causing substring(0) to return the full filename as fileFormat. This leads to incorrect behavior in File.createTempFile().
-String fileFormat = fileName.substring(fileName.lastIndexOf(".") + 1);
-File downloadedFile = File.createTempFile(fileName.replace("." + fileFormat, ""), "." + fileFormat);
+int dotIndex = fileName.lastIndexOf(".");
+String baseName = dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName;
+String fileFormat = dotIndex > 0 ? fileName.substring(dotIndex) : "";
+File downloadedFile = File.createTempFile(baseName, fileFormat.isEmpty() ? ".tmp" : fileFormat);🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/BaseUtilities.java
around lines 54-56, the code assumes a "." exists in fileName; when
lastIndexOf(".") == -1 (no extension) the substring logic yields incorrect
fileFormat and causes File.createTempFile to be called with a malformed suffix.
Fix by checking the dot index first: if dotIndex == -1 set suffix to null or a
safe default like ".tmp" and set prefix to the full filename (ensuring prefix
length >= 3), otherwise split prefix = filename.substring(0, dotIndex) and
suffix = filename.substring(dotIndex) and then call
File.createTempFile(prefixSanitized, suffix). Ensure prefix is sanitized to be
at least 3 chars (pad or truncate) and handle null suffix per
File.createTempFile requirements.
| if (!isFileDownloaded()) { | ||
| throw new RuntimeException("File is still downloading."); | ||
| } |
There was a problem hiding this comment.
Missing download wait logic compared to EdgeUtilities.
ChromeUtilities only performs a single isFileDownloaded() check, while EdgeUtilities uses a WebDriverWait with a 60-second timeout. If a download is still in progress, this will fail immediately instead of waiting for completion.
Apply consistent wait logic:
+import org.openqa.selenium.support.ui.ExpectedCondition;
+import org.openqa.selenium.support.ui.WebDriverWait;
+import java.time.Duration;
driver.get("chrome://downloads/");
-if (!isFileDownloaded()) {
- throw new RuntimeException("File is still downloading.");
-}
+WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(60));
+wait.until((ExpectedCondition<Boolean>) driver -> isFileDownloaded());📝 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 (!isFileDownloaded()) { | |
| throw new RuntimeException("File is still downloading."); | |
| } | |
| import org.openqa.selenium.support.ui.ExpectedCondition; | |
| import org.openqa.selenium.support.ui.WebDriverWait; | |
| import java.time.Duration; | |
| driver.get("chrome://downloads/"); | |
| WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(60)); | |
| wait.until((ExpectedCondition<Boolean>) driver -> isFileDownloaded()); |
🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/ChromeUtilities.java
around lines 25-27, the code currently throws immediately if isFileDownloaded()
is false; replace that single immediate check with the same wait logic used in
EdgeUtilities: create a WebDriverWait with a 60-second timeout and poll until
isFileDownloaded() returns true, then proceed; if the wait times out, throw the
RuntimeException (or rethrow a TimeoutException) with the existing message so
the behavior matches EdgeUtilities.
| public String getDownloadedFileLocalPath() { | ||
| try { | ||
| JavascriptExecutor js = (JavascriptExecutor) driver; | ||
| Object obj = js.executeScript( | ||
| "return document.querySelector('downloads-manager').shadowRoot.querySelector('#downloadsList').items[0].filePath; "); | ||
| assert obj != null; | ||
| return obj.toString(); | ||
| } catch (Exception e) { | ||
| logger.info("No files in the downloads"); | ||
| throw new RuntimeException("No files in the downloads"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Replace assert with explicit null check.
Assertions are disabled by default in production JVMs (-ea flag required). Using assert obj != null; provides no runtime protection.
-assert obj != null;
-return obj.toString();
+if (obj == null) {
+ throw new RuntimeException("No files in the downloads");
+}
+return obj.toString();🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/ChromeUtilities.java
around lines 58 to 69, the code uses an assertion to ensure the
JavascriptExecutor result is not null; assertions are often disabled in
production so replace the assert with an explicit null check: after executing
the script, test if obj == null and if so log a clear message (logger.info or
logger.error) and throw a RuntimeException with a descriptive message
(optionally include context like the executed script or driver state); otherwise
return obj.toString() as before. Ensure the null branch provides actionable
logging and a meaningful exception message.
| List<WebElement> progressValues = (List<WebElement>) ((JavascriptExecutor) driver) | ||
| .executeScript(edgeJavaScript); | ||
| if (progressValues != null && !progressValues.isEmpty()) { |
There was a problem hiding this comment.
Incorrect type cast - JavaScript returns strings, not WebElements.
The JavaScript function returns an array of progress bar values (strings), but the code casts the result to List<WebElement>. This happens to work because only isEmpty() is called, but the type is semantically incorrect and could cause issues if the list contents are accessed.
-List<WebElement> progressValues = (List<WebElement>) ((JavascriptExecutor) driver)
- .executeScript(edgeJavaScript);
+@SuppressWarnings("unchecked")
+List<String> progressValues = (List<String>) ((JavascriptExecutor) driver)
+ .executeScript(edgeJavaScript);🤖 Prompt for AI Agents
In
get_latest_downloaded_file_path/src/main/java/com/testsigma/addons/web/util/EdgeUtilities.java
around lines 84-86, the code incorrectly casts the result of executeScript to
List<WebElement>; the JS returns an array of strings. Change the cast to
List<String> (or List<Object> if values may not be strictly String), rename the
variable accordingly, and update any downstream usage to treat elements as
strings; also keep the null and isEmpty checks but remove any
WebElement-specific operations.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyMjMyMmM2Ni04NWYzLWIyN2UtN2FiOS0zM2U2M2Q4OWM1MGIiLCJ1bmlxdWVJZCI6IjU3NTciLCJpZGVudGl0eUFjY291bnRVVUlkIjoiNDMifQ.YigDc-3C9EQBCE2VMbS6CtsFwE-1ziRJC4zcRFgP0WIUbG5ozAX6QXhrDkVNJOYYgkQ_E11uX-NIokZ6T-00Ig No newline at end of file | |||
There was a problem hiding this comment.
Critical: Hardcoded API key must be removed before merging.
This JWT token is committed to source control and will be exposed in a public repository. API keys and secrets should never be hardcoded in configuration files.
Recommended approaches:
- Use environment variables:
testsigma-sdk.api.key=${TESTSIGMA_API_KEY} - Load from a secrets manager at runtime
- If this is a placeholder for local development, add a
.properties.examplefile instead and add the actual file to.gitignore
Please rotate this key immediately as it may already be compromised once pushed.
🤖 Prompt for AI Agents
In get_latest_downloaded_file_path/src/main/resources/testsigma-sdk.properties
lines 1-1, remove the hardcoded JWT API key and replace it with a reference to
an environment variable (e.g. testsigma-sdk.api.key=${TESTSIGMA_API_KEY}); add a
testsigma-sdk.properties.example with the env-var placeholder for repo and add
testsigma-sdk.properties to .gitignore so real keys are never committed; ensure
application config code reads the env var (or secrets manager) at runtime and
coordinate immediate rotation of the exposed key.
Publish this addon as public
Addon Name: Get latest downloaded file path
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/3072/addons
Jira : https://testsigma.atlassian.net/browse/CUS-9600
Get latest downloaded file path from chrome and edge downloads
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.